-
-
Notifications
You must be signed in to change notification settings - Fork 31
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
Настройка размера батчей и внутреннего предела operating_time #246
Conversation
Reviewer's Guide by SourceryThis pull request addresses issue #241 by introducing new parameters batch_size and operating_time_limit to better manage server load. The changes include updates to the ServerRequestHandler and Bitrix classes to utilize these parameters, as well as corresponding updates to the documentation and example code. File-Level Changes
Tips
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @leshchenko1979 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 4 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟡 Documentation: 5 issues found
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
@@ -35,6 +35,8 @@ | |||
respect_velocity_policy: bool = True, | |||
request_pool_size: int = 50, | |||
requests_per_second: float = 2.0, | |||
batch_size: int = 50, | |||
operating_time_limit: int = 480, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (bug_risk): Consider validating operating_time_limit
.
Validating operating_time_limit
to ensure it is within a reasonable range can prevent potential issues with request throttling.
operating_time_limit: int = 480, | |
operating_time_limit: int = 480, | |
if not (0 < operating_time_limit <= 1440): | |
raise ValueError("operating_time_limit must be between 1 and 1440 minutes") |
Fixes #241
Summary by Sourcery
This pull request introduces enhancements to the Bitrix client by allowing configuration of batch size and operating time limit. Documentation has been updated to reflect these new parameters.