cameronmaske / celery-once

Celery Once allows you to prevent multiple execution and queuing of celery tasks.
https://pypi.python.org/pypi/celery_once/
BSD 2-Clause "Simplified" License
662 stars 90 forks source link

Already queued includes task #38

Closed lackita closed 4 years ago

lackita commented 8 years ago

When AlreadyQueued is raised, occasionally I still need to get at the result it's going to produce. Because the value stored in redis is redundant with the ttl of the entry, I've replaced that value with the result id. This also allowed me to simplify the tests, as the freezegun dependency was not necessary when an actual ttl value was available.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-1.9%) to 96.262% when pulling c18e7cea85e75985798be3b3df49df2bf8345824 on lackita:already-queued-includes-task into 104f2c5c7579909db2d68d81d836a7727964470b on TrackMaven:master.

lackita commented 8 years ago

Strange, I didn't think I had any errors on that branch, I'll take a look and provide an update.

lackita commented 8 years ago

Must be differing versions of python. None > 0 is False in my computer.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-1.9%) to 96.262% when pulling e617b589ddc688e7f556b4d6352465044c012ce9 on lackita:already-queued-includes-task into 104f2c5c7579909db2d68d81d836a7727964470b on TrackMaven:master.

lackita commented 8 years ago

Hmmm, now it's complaining about 30 instead of 30.0, another issue not happening on my system.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-1.9%) to 96.262% when pulling 347522aa8fc08b4f9b602af084b98af0bc8bad76 on lackita:already-queued-includes-task into 104f2c5c7579909db2d68d81d836a7727964470b on TrackMaven:master.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.02%) to 98.131% when pulling 0e822cd242c03a595c4154a14807ccd012e3dfed on lackita:already-queued-includes-task into 104f2c5c7579909db2d68d81d836a7727964470b on TrackMaven:master.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.05%) to 98.165% when pulling 7969d78e641426b125359d4c42aa5bef4c21f195 on lackita:already-queued-includes-task into 104f2c5c7579909db2d68d81d836a7727964470b on TrackMaven:master.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.07%) to 98.182% when pulling 4c6330fac4168aacb7ba2e1828701f0f05c3917b on lackita:already-queued-includes-task into 104f2c5c7579909db2d68d81d836a7727964470b on TrackMaven:master.

TodAmon commented 8 years ago

This would be a nice feature, as I need to obtain information from the task while it is running... regardless of who queued it. Any reason this was not accepted?

lackita commented 8 years ago

@TodAmon I don't think it was a conscious rejection, there are a few other PRs ahead of this one as well.

imomaliev commented 6 years ago

@cameronmaske I think this is better approach, because sometimes you need to know task_id of locked task, for example if you want to add abortable functionality to it, will this be merged?

cameronmaske commented 6 years ago

@imomaliev Apologizes for only getting around to this. There have been significant changes to the underlying code since this was submitted (as can be seen from the merge conflicts), so it need's to be adapted to the current code base. I'll take a look at it once some other PRs are closed out. @lackita Sorry for my silence on this PR, appreciate the contribution and this look vaulable and I'll try and get it master.

lackita commented 6 years ago

@cameronmaske No problem. Tbh, I'm not at the company I wrote this for anymore, so I don't have a good test environment in place to test the resolved code. Will you be able to resolve, or should we try to enlist @imomaliev or @TodAmon?

cameronmaske commented 6 years ago

@lackita I'm in a similar situtation, as I am not at the company I wrote this for anymore too. If anyone wants to submit this PR adapted for the latest code base, it would be a welcome contribution, else I may get around to this in a couple of weeks.

imomaliev commented 6 years ago

cameronmaske I could look into this, also I could become collaborator and take some of the PRs and tickets of your hands

gaetancombes commented 5 years ago

Are there still plans to merge this ? I would really like this feature.

lackita commented 5 years ago

I think the issue is most/all of the people involved in this changed have switched to companies that don't use celery anymore. If you've got an environment where you could resolve the conflicts and test the resulting change, it would go a long way towards getting it merged.

imomaliev commented 5 years ago

@lackita @gaetancombes The problem is that the maintainer of this project rarely checks it. Also after latest changes merged this feature is quiet hard to implement, because locking mechanism has changed

cameronmaske commented 5 years ago

@imomaliev @lackita @gaetancombes Sadly I no longer use celery on a day to day, and don't have much time to maintain this project. This current branch won't be merged as as @lackita pointed out, it's outdated compared to master.

@imomaliev If you'd like to become a contributor, I'd more than welcome it. I'll add you as a collaborator now (I know lots of time has passed since your initial offer, so feel free to decline it). If anyone else is interested, happy to open collaborate access! I'll still handle pushes releases to PyPI (just tag them as a github release/git tag, and make sure the changelog has the details) but can eventually pass those credentials over once the project is in good hands.

imomaliev commented 5 years ago

@cameronmaske Hi, thanks I will help maintain it. Also regarding release we could add https://docs.travis-ci.com/user/deployment/pypi/ so that this project will be automatically published to pypi. After some cleanup we probably should move it jazzband or some other organisation. This is pretty mature project with many users so it should be supported properly

gaetancombes commented 5 years ago

@imomaliev I saw that you are maintaining the repo. Do you have any info/estimate on this particular feature ?

imomaliev commented 5 years ago

@gaetancombes Hi, there was redesign in celery_once and this PR is not relevant anymore, to have this functionality this should be redone

cameronmaske commented 4 years ago

Closing this due to being stale.