generated from NHSDigital/repository-template
-
Notifications
You must be signed in to change notification settings - Fork 4
DTOSS-11680: Standardise completed logging for Commands #745
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8875039 to
22ba978
Compare
Harriethw
commented
Nov 19, 2025
steventux
approved these changes
Nov 20, 2025
Contributor
steventux
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The event monitoring output looks great, very useful as we don't get this without a lot of digging through execution history.
A couple of very minor non-blocking comments. Would merge as is.
manage_breast_screening/notifications/management/commands/helpers/command_handler.py
Outdated
Show resolved
Hide resolved
manage_breast_screening/notifications/management/commands/helpers/command_handler.py
Outdated
Show resolved
Hide resolved
We will shortly refactor this context manager to also log an Insights event if the command finishes successfully - passing the job name will help rather than a specific error.
We will shortly replace the existing exception_handler with this to wrap our Commands.
We want to ensure that shared logic around both exception handling and logging success events are shared across Commands. This extends the existing exception_handler logic.
we need both warning and info
These are now logged to both insights and stdout by the command_handler
to ensure any exception from code will be captured (e.g. if the insights logging call failed)
2706d9d to
c8e60c2
Compare
cleaner name for method
c8e60c2 to
aa26c22
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
We want an easy and consistent way to identify when a job has run successfully
This adds to our existing context handler for Commands and includes a log if no exceptions are found.
I've created a custom event for each command simply because it's easier to see in my opinion on Insights:
Jira link
Review notes
Review checklist