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

Fix bug when ordering elements on queue when get next batch of requests #352

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jpbalarini
Copy link

I found that when using a MySQL database as backend the queue table was being consumed in the opposite order as intended (elements with lower priority were being consumed first).

I found that on the queue component the _order_by method had no default order condition, so I added an explicit desc() clause (elements with bigger priority first). I guess this can apply to other backends (Postgres).

Thanks!

@icapurro
Copy link
Contributor

Great! Thanks for this fix!

@jpbalarini
Copy link
Author

jpbalarini commented Nov 29, 2018

@sibiryakov what do you think of this? Can we merge it?
Thanks!

@sibiryakov
Copy link
Member

hey @jpbalarini thanks for the contribution, will try to respond quicker next time. It seems this wasn't covered with the test, can you add them?

@jpbalarini
Copy link
Author

@sibiryakov done! added tests for the SQL alchemy Queue component. That class should have 100% code coverage now.

Thanks!

@jpbalarini
Copy link
Author

Hi @sibiryakov I added the tests and everything as requested some time ago.
Thanks!

@Prometheus3375
Copy link

When this fix will be applied?

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.

4 participants