[DBMON-6812] Add async job registry to DatabaseCheck - #24442
Conversation
🎉 All green!🧪 All tests passed 🔄 Datadog auto-retried 1 job - 1 passed on retry 🎯 Code Coverage (details) 🔗 Commit SHA: b6d7453 | Docs | Datadog PR Page | Give us feedback! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f1633e741
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| self.tag_manager = TagManager() | ||
| #: Async jobs owned by this check, keyed by job name, populated via | ||
| #: :meth:`register_async_job`. | ||
| self._async_job_registry: Dict[str, "DBMAsyncJob"] = {} |
There was a problem hiding this comment.
Rename the registry attribute without a leading underscore
The root AGENTS.md naming rule says variables may only use a leading underscore for Pydantic PrivateAttrs; this new DatabaseCheck instance attribute is not one of those exceptions. Please rename the registry storage (and the new test references to it) without the underscore so this generated code follows the repo-wide convention.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This was an intentional decision in this case in order to help clarify internal properties for this shared class that will be used by other integrations that we want to discourage leaking logic from. We know it's a soft guard. This is already done across the repo and within this class. If this is new guidance we need to follow from agent integrations then I'm happy to change.
Validation ReportAll 21 validations passed. Show details
|
What does this PR do?
Adds an async job registry to
DatabaseCheckso DBM integrations can declare theirDBMAsyncJobs additively and manage them all through a single entry point instead ofwiring each job's lifecycle by hand.
DatabaseCheckgains:register_async_job(job)— registers a job under itsjob_name, returns it unchanged,no-ops on
None, replaces an existing job registered under the same name, and raisesValueErrorif the job has no name.run_async_jobs(tags)— runs every registered job's loop, forwardingtags.cancel_async_jobs()— signals every job to stop without waiting or releasing resources(safe to call while
check()is running).shutdown_async_jobs()— waits for each job's loop to finish, then runs its teardown.DBMAsyncJobgains:job_nameproperty — public accessor used as the registry key.wait_for_completion()— blocks until the loop finishes and clears its future, hiding theprivate future from callers.
shutdown()— a no-op teardown hook, called once on unschedule after the loop stops, forreleasing lifetime-scoped resources (e.g. dedicated DB connections). Distinct from
shutdown_callback, which runs on every loop exit and may be followed by a restart.Motivation
DBM integrations each hand-roll registering, running, and stopping their async jobs, and
jobs reach into check config directly. Centralizing the registry in
DatabaseCheckgives aconsistent, additive pattern for owning job lifecycles and a clear teardown seam, paving the
way to migrate individual integrations onto it.
Additional Notes
No behavior change for existing integrations; the registry is opt-in and unused until an
integration adopts it.
Review checklist (to be filled by reviewers)