-
Notifications
You must be signed in to change notification settings - Fork 801
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
Additional async-chores for SMTP/IMAP health checks #1812
Additional async-chores for SMTP/IMAP health checks #1812
Conversation
OK, resolve conflicts and I will review. |
@sungam3r Resolved |
Thanks. |
This PR was merged without running tests, see #1018. |
Is there something broken or to do? I ran manual tests and unit tests against a SMTP/IMAP implementation (https://github.com/rnwood/smtp4dev), which doesn't support (STAR)TLS, but basic SMTP and IMAP functionalityand coudln't observe any errors that would not be expected. |
Test step in network CI worflow is disabled https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks/blob/master/.github/workflows/healthchecks_network_ci.yml#L83-L97 I think this is because not properly configured services https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks/blob/master/.github/workflows/healthchecks_network_ci.yml#L32-L62 |
What this PR does / why we need it:
Some chores regarding async methods available in .NET Standard 2.0 and > .NET 5 and passing the cancellation token in operations that could potentially block as discussed in #1739 (comment)
Which issue(s) this PR fixes:
None, but related to #1738
Special notes for your reviewers:
This PR is based on branch https://github.com/peterwurzinger/AspNetCore.Diagnostics.HealthChecks/tree/fix/smtp-timeout
It doesn't make sense to base it on main, since it would result in merge conflicts, assuming that the fix in #1738 would get merged first anyways.
Does this PR introduce a user-facing change?:
No, it only introduces binary breaking changes which shouldn't be a problem according to #1739 (comment)
Please make sure you've completed the relevant tasks for this PR, out of the following list: