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.

Sign up for GitHub

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

Jump to bottom

Use port 25 with starttls instead of 587 for mailing by default #333

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Use port 25 with starttls instead of 587 for mailing by default #333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/vsc/utils/mail.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,10 @@ class VscMail(object):
def __init__(
self,
mail_host='',
mail_port=587,
mail_port=25,
Copy link
Member

Choose a reason for hiding this comment

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

weird defaults (port 587 without starttls makes no sense) sure, but why change them? are you using it somewhere where you can't configure it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, the scripts that mail in vsc-filesystems-quota only use the mail hostname from vsc-config. I could change it to be tuple in vsc-config (hostname, port). But that is a bit more work and needs changes on multiple places.

In my opinion, use port 25 as the default makes more sense? I also don't really see good reason to use 587 as long as you use starttls?

Copy link
Member

Choose a reason for hiding this comment

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

well, port 25 is not supposed to be used for client-mailserver, only mailserver-mailserver.
it's best if we can somehow configue it properly (even enabling/disabling starttls)

Copy link
Member

Choose a reason for hiding this comment

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

@wpoely86 perhaps allow the smtp option to indicate a filename with the mailconfig if it starts with a /?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually upon better reading the code, we could just provide server:25 and that will also work.

But I would suggest that we flip on starttls by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively we can add a default configuration file and gracefully handle it if it's not present?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is 25 only server-server? Didn't know that. I thought the only difference was that 587 enforces TLS.

smtp_auth_user=None,
smtp_auth_password=None,
smtp_use_starttls=False,
smtp_use_starttls=True,
mail_config=None):
"""
- If there is a config file provided, its values take precedence over the arguments passed to __init__
Expand Down
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
]

PACKAGE = {
'version': '3.4.9',
'version': '3.4.10',
'author': [sdw, jt, ag, kh],
'maintainer': [sdw, jt, ag, kh],
# as long as 1.0.0 is not out, vsc-base should still provide vsc.fancylogger
Expand Down