Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[APIPUB-72] Develop solution for processing rate limited/left over messages when warned #74

Merged
merged 5 commits into from
Sep 17, 2024

Conversation

jleiva-gap
Copy link
Contributor

Add retry policy for rate limiting

…ssages when warned

Add retry policy for rate limiting
Add new option to documentation
Copy link

github-actions bot commented Sep 13, 2024

Test Results

173 tests   173 ✅  5s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit ed2a67d.

♻️ This comment has been updated with latest results.

@jleiva-gap jleiva-gap marked this pull request as ready for review September 13, 2024 15:58
throw new Exception(
"Unable to reduce resource dependencies for processing key changes as expected (the infinite loop threshold was exceeded during processing).");
}
//if (i == infiniteLoopProtectionThreshold)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed that, we need the code

@DavidJGapCR
Copy link
Contributor

DavidJGapCR commented Sep 17, 2024

There are some warning with this message
This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
Should we fix this now or fix it as part of APIPUB-76 ?

@@ -29,6 +29,7 @@ Defines general behavior of the Ed-Fi API Publisher.
| Options:EnableRateLimit<br/>`--enableRateLimit` | Indicates whether or not to use rate limiting.<br/>(_Default value: false_) |
| Options:RateLimitNumberExecutions<br/>`--rateLimitNumberExecutions` | Indicates the maximum number of executions allowed within the defined time window.<br/>(_Default value: 100_) |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed that this documentation says default value is 100. But in the app Settings file it changed to 30.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@DavidJGapCR DavidJGapCR merged commit e2e0742 into main Sep 17, 2024
7 checks passed
@DavidJGapCR DavidJGapCR deleted the APIPUB-72 branch September 17, 2024 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants