Skip to content

Periodic task querying is a separate method#883

Merged
auvipy merged 1 commit into
celery:mainfrom
maciej-gol:mgol/schedules-separate-method
May 11, 2025
Merged

Periodic task querying is a separate method#883
auvipy merged 1 commit into
celery:mainfrom
maciej-gol:mgol/schedules-separate-method

Conversation

@maciej-gol

Copy link
Copy Markdown
Contributor

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
@codecov

codecov Bot commented May 4, 2025

Copy link
Copy Markdown

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).
Report is 3 commits behind head on main.

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

@auvipy auvipy left a comment

Copy link
Copy Markdown
Member

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

maciej-gol commented May 5, 2025 via email

Copy link
Copy Markdown
Contributor Author

@auvipy

auvipy commented May 5, 2025

Copy link
Copy Markdown
Member

Some Additional tests would be nice

@auvipy

auvipy commented May 8, 2025

Copy link
Copy Markdown
Member

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

also we have another PR #886

@alirafiei75

Copy link
Copy Markdown
Contributor

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

also we have another PR #886
This PR does not introduce any behavioral change and it seems OK.

@auvipy auvipy merged commit d78ba7a into celery:main May 11, 2025
27 of 28 checks passed
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.

3 participants