Skip to content

Periodic task querying is a separate method #883

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

maciej-gol
Copy link

Recent changes to how periodic tasks are filtered and fetched broke how tenant-schemas-celery fetch all periodic tasks from all schemas.

In order not to copy-paste or duck-type new behavior, the filtering logic has been refactored to a separate method, enabled_models_qs(). This method does what the original query would do. Next, the queryset is consumed into a list inside enabled_models(), which is also used in the old for-loop.

The enabled_models() method is a good integration point, as we still have access to the unevaluated queryset there, whilst being able to return simple datastructure which is a list.

In my case, I would use enabled_models_qs() to construct the query, and run it against all of the schemas. The result of enabled_models() will be the union of all the results across all of the schemas.

@maciej-gol maciej-gol force-pushed the mgol/schedules-separate-method branch from 908d76b to d51be79 Compare May 4, 2025 08:44
Copy link

codecov bot commented May 4, 2025

Codecov Report

Attention: Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 88.11%. Comparing base (87c0597) to head (d51be79).

Files with missing lines Patch % Lines
django_celery_beat/schedulers.py 80.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #883      +/-   ##
==========================================
+ Coverage   88.07%   88.11%   +0.04%     
==========================================
  Files          32       32              
  Lines        1006     1010       +4     
  Branches      104      104              
==========================================
+ Hits          886      890       +4     
  Misses        101      101              
  Partials       19       19              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@auvipy auvipy self-requested a review May 5, 2025 08:14
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.

thanks for the patch. can you please also add some sort of failing test for this change?

@maciej-gol
Copy link
Author

maciej-gol commented May 5, 2025 via email

@auvipy
Copy link
Member

auvipy commented May 5, 2025

Some Additional tests would be nice

@auvipy
Copy link
Member

auvipy commented May 8, 2025

@alirafiei75 can you take a look into it as well please?

also we have another PR #886

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.

2 participants