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

Feature: save_limit per task group #508

Closed

Conversation

jonaswinkler
Copy link

This implements the idea presented in #491.

To summarize the issue:

  • I've got tasks that execute every 10 minutes. (group1)
  • I've got tasks that execute once every week. (group2)
  • I'd like to keep success messages of both groups in the database, without configuring save_limit to something very high (10000+ would be required here). It's also somewhat annoying that making a sane configuration here depends on the actual number of groups and their schedule.

Impact:

  • Nothing changes with the default configuration.
  • If save_limit is 0, and save_limit_per_group is greater than zero, save_limit_per_group takes effect.
  • If both are greater than zero, both take effect at the same time, deleting old results whenever either save_limit or save_limit_per_group is exceeded.

Open points:

  • This considers tasks with group=None to be one group as well.

If this is something you want, I'll also add relevant test cases and documentation.

@codecov-io
Copy link

codecov-io commented Feb 14, 2021

Codecov Report

Merging #508 (73e2278) into master (e258005) will decrease coverage by 0.02%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #508      +/-   ##
==========================================
- Coverage   90.54%   90.52%   -0.03%     
==========================================
  Files          47       47              
  Lines        3088     3091       +3     
==========================================
+ Hits         2796     2798       +2     
- Misses        292      293       +1     
Impacted Files Coverage Δ
django_q/cluster.py 91.55% <50.00%> (-0.19%) ⬇️
django_q/conf.py 73.68% <100.00%> (+0.23%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e258005...73e2278. Read the comment docs.

@Koed00
Copy link
Owner

Koed00 commented Feb 26, 2021

Before I merge this, could you add a small entry in the documentation for this new option? Otherwise people will never find it.
Thanks for the work!

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