-
Notifications
You must be signed in to change notification settings - Fork 2
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
LTD-4645-healthcheck-app-readd #229
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #229 +/- ##
==========================================
+ Coverage 80.70% 81.01% +0.31%
==========================================
Files 55 55
Lines 3120 3119 -1
Branches 432 432
==========================================
+ Hits 2518 2527 +9
+ Misses 512 502 -10
Partials 90 90 ☔ View full report in Codecov by Sentry. |
PendingMailHealthCheck, | ||
) | ||
|
||
plugin_dir.register(MailboxAuthenticationHealthCheck) | ||
plugin_dir.register(LicencePayloadsHealthCheck) | ||
plugin_dir.register(ManageInboxTaskHealthCheck) |
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.
Is the code for ManageInboxTaskHealthCheck
still present? Would suggest removing it if so
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.
ah no, it was removed when @Tllew merged his PR
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.
I'm not sure how you can check the manageinbox task as it's all converted to celery, but if it works then happy to keep it in
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.
We decided to drop the task, but a little confused that it's still referenced here
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.
Yeah it is a bit confusing to see these references but the code is not there.
PendingMailHealthCheck, | ||
) | ||
|
||
plugin_dir.register(MailboxAuthenticationHealthCheck) | ||
plugin_dir.register(LicencePayloadsHealthCheck) | ||
plugin_dir.register(ManageInboxTaskHealthCheck) |
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.
I'm not sure how you can check the manageinbox task as it's all converted to celery, but if it works then happy to keep it in
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.
Suggest deploying to an env and checking before merging this.
done checked on UAT and DEMO both working fine |
a bit of cleaning up since few things were removed when merged.