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

scrapinghub / frontera

A scalable frontier for web crawlers

BSD 3-Clause "New" or "Revised" License

1.29k stars 216 forks source link

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

Open jpbalarini opened 5 years ago

jpbalarini commented 5 years ago

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 commented 5 years ago

Great! Thanks for this fix!

jpbalarini commented 5 years ago

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

sibiryakov commented 5 years ago

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 commented 5 years ago

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

Thanks!

jpbalarini commented 5 years ago

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

Prometheus3375 commented 4 years ago

When this fix will be applied?