Skip to content
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

Support for smtp vs smtps #129

Closed
SanjulaGanepola opened this issue Jul 19, 2024 · 1 comment · Fixed by #130
Closed

Support for smtp vs smtps #129

SanjulaGanepola opened this issue Jul 19, 2024 · 1 comment · Fixed by #130
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@SanjulaGanepola
Copy link
Collaborator

After the change made in #126 to go from smtp to smtps, I noticed that we can no longer use a server such as na.relay.ibm.com. This is because of the port that it ends up using:

  • smtp -> 25
  • smtps -> 465

Option 1: Support both smtp and smtp as a destination <id>.
Option 2: Support only smtp as the <id> and add an optional field for the port where the default is 25.

@ThePrez @jonnyz32 What are your thoughts on this?

I think option 2 is probably better since users can set whatever SMTP port they want based on what they have configured as it looks like there are even a few more typical SMTP ports:
image

@SanjulaGanepola SanjulaGanepola added bug Something isn't working help wanted Extra attention is needed labels Jul 19, 2024
@SanjulaGanepola SanjulaGanepola self-assigned this Jul 19, 2024
@ThePrez
Copy link
Owner

ThePrez commented Jul 19, 2024

Thanks for catching, Sanjula!

I think we need to do Option 1, simply because it's hypothetically possible to host smtp on 465 and smtps on 25, etc.
From a code perspective, it's a matter of just duplicating the logic for smtp and smtps and allowing for port configuration (I think that's already done)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants