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

Allow plugin authors to register housekeeping tasks #16971

Closed
jsenecal opened this issue Jul 24, 2024 · 9 comments · Fixed by #17716
Closed

Allow plugin authors to register housekeeping tasks #16971

jsenecal opened this issue Jul 24, 2024 · 9 comments · Fixed by #17716
Assignees
Labels
complexity: low Requires minimal effort to implement netbox status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Milestone

Comments

@jsenecal
Copy link
Contributor

NetBox version

v4.0.7

Feature type

New functionality

Proposed functionality

NetBox includes a housekeeping management command that usually runs nightly. This command handles:

  • Clearing expired authentication sessions from the database
  • Deleting older changelog records
  • Deleting older job result records
  • Checking for new NetBox releases

It would be great if plugin authors could register additional tasks to run by the housekeeping process without having to write their own management commands and setup mechanisms to run those periodically.

Use case

I often have to write periodic tasks to alter model instances in my plugins, reacting to dates, sending emails etc. and having to setup the whole management commands and related mechanisms to run those periodically is not really convenient, even more so considering that NetBox already has mechanisms built in to do so.

Database changes

None whatsoever

External dependencies

None

@jsenecal jsenecal added status: needs triage This issue is awaiting triage by a maintainer type: feature Introduction of new functionality to the application labels Jul 24, 2024
@alehaa
Copy link
Contributor

alehaa commented Jul 25, 2024

The background jobs framework of #15692 will include the ability to schedule system jobs, i.e. housekeeping or other tasks that run periodically in the background. While the plugin still requires a special management command for interactive execution (optional), periodic execution is covered by this framework.

@jeremystretch
Copy link
Member

I think ultimately we probably want to do away with the separate housekeeping task and integrate everything using the job's framework, particularly in light of @alehaa's recent work.

@jeremystretch jeremystretch removed their assignment Jul 25, 2024
@jeremystretch jeremystretch added status: under review Further discussion is needed to determine this issue's scope and/or implementation and removed status: needs triage This issue is awaiting triage by a maintainer labels Jul 25, 2024
@alehaa
Copy link
Contributor

alehaa commented Jul 30, 2024

The JobRunner framework now supports the implementation of any kind of task, including housekeeping. However, the automatic registration of jobs on plugin load did not make it into the PR because the implementation is not stable enough for production. I suggest this FR to focus on this remaining issue, so that even housekeeping tasks can be set up automatically and solve this problem.

Here are the latest findings:

  • Calling JobRunner.enqueue_once() directly from the ready() method of AppConfig logs a warning because the database is accessed during plugin setup. The Django documentation explicitly warns against this use case, as the ready() method is called every time a plugin is loaded, even if it is only for the nbshell management command.
  • Using the connection_created signal solves the console warning. However, this triggers the JobRunner setup even more often, e.g. because the database is connected before each job execution.

So the problem for now is to find a way to automatically trigger the setup method outside of the request-response cycle that is called only once or say every few hours.

@alehaa
Copy link
Contributor

alehaa commented Jul 30, 2024

How about adding the auto-registration to the beginning of the rqworker management command? It would only be called once for each worker started - not for the frontend and not for each database connection. After updates, restarting the rqworker would trigger auto-registration again.

I'd implement this using a new method in AppConfig (e.g. enqueue_job_runner()) being called by rqworker.handle() for each plugin. An alternative would be using a signal, which avoids a new method but could obfuscate the logic.

@jsenecal
Copy link
Contributor Author

How about adding the auto-registration to the beginning of the rqworker management command? It would only be called once for each worker started - not for the frontend and not for each database connection. After updates, restarting the rqworker would trigger auto-registration again.

I'd implement this using a new method in AppConfig (e.g. enqueue_job_runner()) being called by rqworker.handle() for each plugin. An alternative would be using a signal, which avoids a new method but could obfuscate the logic.

This feels like a decent approach.
Netbox also has existing mechanisms working with "registries"; we could have a "Housekeeping tasks" registry which would be handled in a similar fashion by the worker and would provide a standardized way to register periodic tasks.

@alehaa
Copy link
Contributor

alehaa commented Aug 1, 2024

I'm not sure if this is possible. Usually the registry handles static variables like a list of middleware to load. To register a JobRunner, its enqueue_once() method needs to be called and its arguments can vary depending on the plugin's implementation. Therefore I think some kind of method needs to be called for each plugin, but of course it can reside in the AppConfig class along with other registry-related variables. In this method, plugins could implement the logic to setup the JobRunner, i.e. by loading configuration or scheduling task depending on other NetBox objects, etc.

@alehaa
Copy link
Contributor

alehaa commented Aug 16, 2024

@jeremystretch can you assign this to me so I can work on a PR? Maybe we can still make it into 4.1.

@alehaa
Copy link
Contributor

alehaa commented Sep 24, 2024

@jeremystretch did you have any chance to review this? This would be the last missing piece to add auto-scheduling background tasks, which didn't make it into the JobRunner framework.

@DanSheps DanSheps added status: accepted This issue has been accepted for implementation complexity: low Requires minimal effort to implement and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Oct 2, 2024
@DanSheps
Copy link
Member

DanSheps commented Oct 2, 2024

@alehaa I am not so sure about the approach, I think something more agnostic that can be called after initialization of all apps are done but not specifically tied jobs or RQ might be a better approach.

That said, I don't know if there is a viable solution for this as there is no "apps_ready" signal emitted by django. Your solution might be the most viable right now.

@jeremystretch jeremystretch added the netbox label Nov 1, 2024 — with Linear
jeremystretch added a commit that referenced this issue Nov 1, 2024
* Fix check for existing jobs

If a job is to be enqueued once and no specific scheduled time is
specified, any scheduled time of existing jobs will be valid. Only if a
specific scheduled time is specified for 'enqueue_once()' can it be
evaluated.

* Allow system jobs to be registered

A new registry key allows background system jobs to be registered and
automatically scheduled when rqworker starts.

* Test scheduling of system jobs

* Fix plugins scheduled job documentation

The documentation reflected a non-production state of the JobRunner
framework left over from development. Now a more practical example
demonstrates the usage.

* Allow plugins to register system jobs

* Rename system job metadata

To clarify which meta-attributes belong to system jobs, each of them is
now prefixed with 'system_'.

* Add predefined job interval choices

* Remove 'system_enabled' JobRunner attribute

Previously, the 'system_enabled' attribute was used to control whether a
job should run or not. However, this can also be accomplished by
evaluating the job's interval.

* Fix test

* Use a decorator to register system jobs

* Specify interval when registering system job

* Update documentation

---------

Co-authored-by: Jeremy Stretch <jstretch@netboxlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: low Requires minimal effort to implement netbox status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants