Added support for requests that contain an ETA datetime in the future

[weetster]

Hello,

I'm using celery-batches for a project that requires failed tasks to be retried at a later time (for example, if a web API request fails). I documented the approach that I was hoping to use in the case of failure and retry (apply_async(countdown=10)) in this pull request, which was inspired by the discussion in issue #10. I noticed that the approach did not work, because [email protected] does not handle Celery Request objects that have an .eta field. I've added support to celery-batches for handling this scenario, and I hope that you will consider merging my changes into the main branch for inclusion in an upcoming 0.7 release.

Thank you.

[clokep]

Thanks! Will try to take a look soon! 👍

[clokep]

Looks like there's a couple of lint errors, could you run flake8 (or take a look at the CI output) and fix those up? Thanks!

[weetster]

Hey, thanks for the code review. I just noticed it now, since it was posted the same day that I left for a week-long vacation. I'll take a look at addressing the comments as soon as I can.

[clokep]

Hey, thanks for the code review. I just noticed it now, since it was posted the same day that I left for a week-long vacation. I'll take a look at addressing the comments as soon as I can.

No problem! Do let me know if you have any questions or disagree with any of the comments. I think I also bitrotted this PR decently by adding type hints, sorry about that. 😢 Hopefully they make it a bit easier to see what's going on though!

[clokep]

I need to document this, but you should be able to do the following to run the lint checks:

$ pip install -r requirements/pkgutils.txt
$ flake8
$ isort
$ black
$ mypy

(And don't worry about random extra commits -- we'll squash merge anyway.)

[weetster]

I was just making fixes for the workflow, didn't see your new comments until now. Thanks, will take a look!

[weetster]

All set for another look. Thanks again!

[clokep]

@weetster This is back to you, note that if you can't get the tests to run with live brokers yet, we can skip it. I think there's something flakey with returning results in those cases. (You can see they skip another test also...)

[weetster]

@weetster This is back to you, note that if you can't get the tests to run with live brokers yet, we can skip it. I think there's something flakey with returning results in those cases. (You can see they skip another test also...)

Thanks, the tests work well on my local machine, so I added a skip similar to the other test case that you mentioned.

[clokep]

If it is easier I can make the remaining changes and push them to your branch (I think I have the right permissions to do that...)

[weetster]

If it is easier I can make the remaining changes and push them to your branch (I think I have the right permissions to do that...)

Sure, that's ok with me, if it's easier for you to make the changes to how you want it. Let me know if there's anything else I should do. 👍

[clokep]

@weetster I pushed a few commits to:

• Update the tests a bit like we discussed.
• Update the documentation (to be more inline with other changes on main).
• Properly handle timezones (mostly be copying the behavior inside of Celery's default strategy).

Does this all look reasonable to you? If you could give it a quick test and make sure it still works for you I would appreciate it! 👍 The current release already has a lot in it so would like to include this and then get it out the door.

[weetster]

@clokep Thanks for the extra commits, good to handle timezones properly. celery-batches unit tests pass for me, and my project that relies on celery-batches also passes its unit tests with the latest changes. Looks good to me for a release.

[clokep]

Thanks for checking! 🎉 (And for making this PR!)

I plan to do a new release soon!

[weetster]

You're welcome, and thanks for accepting the changes! They will be a huge help to me and hopefully others.