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 superseded ExtendedQuerySet as it's functionality is built into Django since 1.7 #563

Merged
merged 3 commits into from Oct 13, 2022

Conversation

lociii
Copy link
Contributor

@lociii lociii commented Jul 8, 2022

Fixes #562

ExtendedQuerySet provided a custom implementation of update_or_create before the release of Django 1.7. Since then Django added it to the base QuerySet.
Only difference is that update_or_create of Django's QuerySet returns the object and creation state whereas the custom implementation of django-celery-beat only returned the object. Therefore ModelEntry.from_entry had to be adjusted.

As a cleanup, I removed all managers and introduced a single queryset for PeriodicTask.

@auvipy auvipy self-requested a review July 8, 2022 15:39
Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

builds are failing. also some additional tests would be great

@lociii
Copy link
Contributor Author

lociii commented Jul 11, 2022

builds are failing. also some additional tests would be great

Builds are fixed now as far as I can influence them.
What kind of tests are you looking for? I can test PeriodicTaskQuerySet but that seems pretty pointless to me.

@auvipy
Copy link
Member

auvipy commented Jul 11, 2022

it's better to verify with automated test

@auvipy
Copy link
Member

auvipy commented Oct 12, 2022

can you add a little test for the queryset?

@954-Ivory
Copy link
Contributor

can you add a little test for the queryset?

This PR just use django native model function, to replace custom function.
It should not to add additional test code.

I think this PR is better in django version>1.7:
Because it will call native function, the native function will call with transaction.atomic, it's safer.

But I don't know does django has update_or_create in version <1.7.
If there has not update_or_create, some user use django-celery-beat with django<1.7 will got the error.

@auvipy
Copy link
Member

auvipy commented Oct 13, 2022

this will be django 2.2+ only

@auvipy auvipy merged commit 626c2a0 into celery:master Oct 13, 2022
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.

ExtendedQuerySet.update_or_create violates signature of parent method
3 participants