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

Remove unnecessary atomic decorators from tasks #1720

Merged
merged 2 commits into from
Oct 18, 2023

Conversation

mvandenburgh
Copy link
Member

I don't think the atomic decorator actually does anything for these task functions, but please correct me if I'm missing something @danlamanna @AlmightyYakob.
Any complex operations that do need to be atomic are already wrapped in an atomic block inside the service layer functions that the tasks are calling.

The `atomic` decorator isn't actually needed on these task functions.
Any parts of the tasks that need to be atomic are already marked as
such inside the functions that the tasks call.
@mvandenburgh
Copy link
Member Author

mvandenburgh commented Oct 18, 2023

Looks like the atomic decorator was actually doing something on the publish task 😅 - this select_for_update call is erroneously made outside of the atomic block, but it went unnoticed until now since the celery task had the atomic decorator. a5b5786

(this is why the CLI tests are failing)

@mvandenburgh mvandenburgh merged commit dec4409 into master Oct 18, 2023
10 checks passed
@mvandenburgh mvandenburgh deleted the remove-unneeded-atomic branch October 18, 2023 20:41
@dandibot
Copy link
Member

🚀 PR was released in v0.3.58 🚀

@dandibot dandibot added the released This issue/pull request has been released. label Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants