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

Соблюдение новых ограничений на длительность запросов #217

Merged
merged 3 commits into from
Mar 2, 2024

Conversation

leshchenko1979
Copy link
Owner

Fixes #187

Copy link

codecov bot commented Mar 2, 2024

Codecov Report

Attention: Patch coverage is 85.18519% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 84.05%. Comparing base (754de24) to head (5d398a8).

Files Patch % Lines
fast_bitrix24/srh.py 62.50% 6 Missing ⚠️
fast_bitrix24/leaky_bucket.py 96.77% 1 Missing ⚠️
fast_bitrix24/logger.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #217      +/-   ##
==========================================
- Coverage   89.32%   84.05%   -5.27%     
==========================================
  Files          10       11       +1     
  Lines         515      533      +18     
==========================================
- Hits          460      448      -12     
- Misses         55       85      +30     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

PR Type: Enhancement

PR Summary: This pull request introduces a new feature aimed at enforcing new request duration limits within the application. It includes the addition of a LeakyBucketLimiter class to manage request rates effectively, ensuring that the application adheres to specified constraints on the duration and frequency of requests. The changes span across multiple files, including the introduction of new tests to validate the functionality of the limiter and modifications to existing request handling logic to incorporate the new rate limiting strategy.

Decision: Comment

📝 Type: 'Enhancement' - not supported yet.
  • Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
✅ Small diff: the diff is small enough to approve with confidence.
No details provided.

General suggestions:

  • Given the significant changes introduced, especially with the addition of method-specific rate limiting, it's crucial to ensure comprehensive documentation is in place. This will help users and future maintainers understand the rationale behind these changes and how to work with the new rate limiting features.
  • Consider simplifying the implementation if possible to reduce complexity. While the introduction of a LeakyBucketLimiter provides a robust solution for rate limiting, the added complexity could make the system harder to maintain and extend in the future.
  • Ensure thorough testing of the new rate limiting logic under various scenarios, including high load and edge cases, to guarantee the system behaves as expected without introducing regressions.

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

fast_bitrix24/srh.py Show resolved Hide resolved
fast_bitrix24/srh.py Show resolved Hide resolved
tests/test_async.py Show resolved Hide resolved
fast_bitrix24/srh.py Show resolved Hide resolved
fast_bitrix24/srh.py Show resolved Hide resolved
tests/test_leaky_bucket.py Show resolved Hide resolved
tests/test_leaky_bucket.py Show resolved Hide resolved
tests/test_leaky_bucket.py Show resolved Hide resolved
@leshchenko1979 leshchenko1979 self-assigned this Mar 2, 2024
@leshchenko1979 leshchenko1979 merged commit c1e8569 into master Mar 2, 2024
9 of 11 checks passed
@leshchenko1979 leshchenko1979 deleted the leshchenko1979/issue187 branch March 2, 2024 19:28
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.

Соблюдение новых ограничений на длительность запросов
1 participant