-
-
Notifications
You must be signed in to change notification settings - Fork 974
Add support for fair queue in SQS #2342
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR adds support for AWS SQS fair queues by allowing MessageGroupId to be sent for standard (non-FIFO) queues. The change ensures compatibility with AWS's recently introduced native fair queueing mechanism.
- Modifies the SQS transport logic to check for MessageGroupId independently of FIFO queue detection
- Restructures conditional logic to handle both fair queues and FIFO queues appropriately
- Adds test coverage for MessageGroupId functionality in standard queues
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| kombu/transport/SQS.py | Updates the _put method to support MessageGroupId for both standard and FIFO queues |
| t/unit/transport/test_SQS.py | Adds test case to verify MessageGroupId is properly passed through for standard queues |
|
@auvipy , Could you share the plan here? When are we planning to release this feature? I see it has been added to 5.7.0 milestone. Do we have any date in mind? Is it possible to make it part of 5.6.0, if the change is not that big? |
|
it will be slated for v5.70 |
@auvipy Do we know any appropriate time when it would be released? also, will be update the previous versions with this feature? |
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.
we can consider this for 5.7 release as well, but need more test coverage
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.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@auvipy more test coverage has been added, thanks to @vaibhavcerta |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2342 +/- ##
==========================================
+ Coverage 81.09% 81.23% +0.13%
==========================================
Files 77 77
Lines 9734 9735 +1
Branches 1098 1099 +1
==========================================
+ Hits 7894 7908 +14
+ Misses 1631 1623 -8
+ Partials 209 204 -5 ☔ View full report in Codecov by Sentry. |
auvipy
left a comment
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.
also please fix the lint issues
| to `basic_publish` method. | ||
|
|
||
| Fair Queue Support | ||
| ------------------------ |
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.
will need a versionadded 5.7 annotation here
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.
@auvipy both issues are taken care of, please check and let me know
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.
reminder on this @auvipy
AWS SQS recently introduced native support for fair queues (reference).
To enable this functionality, we need to send the MessageGroupId even for standard (non-FIFO) queues. This change ensures compatibility with the new fair queueing mechanism.