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

Task name always include CRON #7

Closed
Tobi-De opened this issue Feb 10, 2024 · 4 comments
Closed

Task name always include CRON #7

Tobi-De opened this issue Feb 10, 2024 · 4 comments

Comments

@Tobi-De
Copy link

Tobi-De commented Feb 10, 2024

image

Regardless of the type of schedule used, the task name always includes CRON.

@joshuadavidthomas
Copy link
Member

joshuadavidthomas commented Feb 11, 2024

Hold over from when this was just an internal thing I copied from project to project.

There's no good way to mark a scheduled task as part of the registry. The quick and easy way I came up with was to make sure the name had some identifiable string that we can filter on, so that when the setup_periodic_tasks management command is called the registry knows which scheduled tasks its responsible for.

There has to be a better way of accomplishing this -- quick brainstorm, a model specific to this app that has a ForeignKey to the Schedule it's tied to would be a bit more robust, and keep the task name from being polluted with the extra string appended at the end.

For now, that suffix could be renamed to something a bit more generic, perhaps "- QREGISTRY" or something else.. Open to suggestions here.

In the meantime, it's actually configurable via the Q_REGISTRY["PERIODIC_TASK_SUFFIX"] setting.

@joshuadavidthomas
Copy link
Member

Also, Falco is super cool! I've definitely looked at in the past, but went with my own template because, you know, I've got Opinions. 😄

I saw that you name checked django-twc-project, thanks for that! 🎉 🍻

@Tobi-De
Copy link
Author

Tobi-De commented Feb 11, 2024

Also, Falco is super cool! I've definitely looked at in the past, but went with my own template because, you know, I've got Opinions. 😄

I saw that you name checked django-twc-project, thanks for that! 🎉 🍻

Understandable 😄
django-twc-project really has some great ideas in it, I'm planning to steal some more :')

@Tobi-De
Copy link
Author

Tobi-De commented Feb 11, 2024

Hold over from when this was just an internal thing I copied from project to project.

There's no good way to mark a scheduled task as part of the registry. The quick and easy way I came up with was to make sure the name had some identifiable string that we can filter on, so that when the setup_periodic_tasks management command is called the registry knows which scheduled tasks its responsible for.

There has to be a better way of accomplishing this -- quick brainstorm, a model specific to this app that has a ForeignKey to the Schedule it's tied to would be a bit more robust, and keep the task name from being polluted with the extra string appended at the end.

For now, that suffix could be renamed to something a bit more generic, perhaps "- QREGISTRY" or something else.. Open to suggestions here.

In the meantime, it's actually configurable via the Q_REGISTRY["PERIODIC_TASK_SUFFIX"] setting.

Simple is better than complex. I think setting QREGISTRY as the default is good enough; it's simple and explicit. But you just made me realize something: if I manually create a task in the admin myself, setup_periodic_tasks won't drop it at the next run? Am I right? If that's the case, that's great news and could be a good idea to mention this in the readme.

@Tobi-De Tobi-De closed this as completed Feb 11, 2024
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

No branches or pull requests

2 participants