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

Enable configuration of result_ttl in django-rq-scheduler #18

Merged
merged 4 commits into from
Jul 10, 2018
Merged

Enable configuration of result_ttl in django-rq-scheduler #18

merged 4 commits into from
Jul 10, 2018

Conversation

frbry
Copy link
Contributor

@frbry frbry commented Sep 3, 2017

Currently all scheduled and repeatable tasks are created with result_ttl=-1. This causes Result will never expire, clean up result key manually warnings to be logged, which can be annoying.

This PR creates a new field in BaseTask model so the user has the ability to change result_ttl value for newly created RQ tasks.

Note: result_ttl defaults to -1, which is weird because the RQ docs say it's 500 seconds by default: http://python-rq.org/docs/results/

@frbry
Copy link
Contributor Author

frbry commented Oct 28, 2017

Hi @g3rd, any chance you could take a look at this one? Thanks! :)

@frbry
Copy link
Contributor Author

frbry commented Mar 9, 2018

Hi @g3rd, any feedback?

@oudeismetis
Copy link
Contributor

@frbry
Sorry for the slow reply on this.
At a high level this looks good. Going to pull this down and check it out, but I expect this will be merged soon.

Thanks for thinking of this!

@tom-price tom-price mentioned this pull request Jun 29, 2018
@tom-price
Copy link
Collaborator

@oudeismetis, I've created PR #22 to merge @frbry's work into the project as there were minor issues related to the introduction of cron jobs.

@oudeismetis
Copy link
Contributor

@frbry thank you again for the contribution!
Given the merge conflict, we are looking to merge #22 to bring this feature in.
Once that is merged I'll close this PR.

@frbry
Copy link
Contributor Author

frbry commented Jul 3, 2018

Thanks @oudeismetis and @tom-price!

@bashhack bashhack merged commit e93c740 into islco:master Jul 10, 2018
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.

None yet

4 participants