Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix STARTTLS to use port 587 #41

Merged
merged 2 commits into from
Oct 7, 2022
Merged

Fix STARTTLS to use port 587 #41

merged 2 commits into from
Oct 7, 2022

Conversation

drfraser
Copy link
Contributor

@drfraser drfraser commented Oct 6, 2022

SMTPType.STARTTLS was set to 465 as its default port - it is actually 587. Tests and the mock SMTPMock object were also changed because this uncovered a bug aka a code path that wasn't actually ever tested.

Closes #39

Checklist

  • This pull request references any related issue by including "Closes #<ISSUE_NUMBER>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • This pull request includes tests or only affects documentation.
  • Summarized PR's changes in CHANGELOG.md

Copy link
Contributor

@ahuang11 ahuang11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks this looks good! I think it just needs an entry in the CHANGELOG.md

@ahuang11 ahuang11 merged commit 7a3b98a into PrefectHQ:main Oct 7, 2022
@ahuang11
Copy link
Contributor

ahuang11 commented Oct 7, 2022

Thanks for fixing this @drfraser!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SSL and STARTTLS are not equivalent
2 participants