Alternative to PR 669 #675
Conversation
1. Respect empty CC and BCC and do not deliver <> to SMTP Server 2. Allow TLS also on non-SSL sessions 3. Turn on debugging before TLS to see that log as well 4. Check for parameter `files` and empty if missed 5. Loop through files and append to attachments 6. Allow Attachment Disposition Name differ from Filename by path/filename.ext=attname.ext 7. Allow body to send in text and html by list or dict
Fix linter issues
Fix Linter
…berg/cement into pr-669
- Resolves Pull Request 669 - Resolves Issue 667 - Resolves Issue 668
|
@TomFreudenberg this PR is based on the work you did in #669. Curious if you might review the changes and confirm that it suits your needs. Some minor changes:
|
|
Hi @derks I will check that today and come up with my response. In general it looks fine so far. Thanks for your invest on that! |
cement/ext/ext_smtp.py
Outdated
| part = MIMEBase('application', 'octet-stream') | ||
|
|
||
| # support for alternative file name if its tuple | ||
| # like (filename.ext=attname.ext) |
There was a problem hiding this comment.
Comment should be
# like ['filename.ext', 'attname.ext']There was a problem hiding this comment.
In case you change the order in the list,
the comment must be:
# like ['attname.ext', 'path/filename.ext']| from ..utils.misc import minimal_logger, is_true | ||
|
|
||
|
|
||
| LOG = minimal_logger(__name__) |
There was a problem hiding this comment.
Hi @derks
why do you prefer to create a seperate LOG in each library than using the app.log general instance?
I prefer using self.app.log.debug() - am I wrong?
There was a problem hiding this comment.
@TomFreudenberg extensions do not use the app logger by design, I can't recall exactly but was probably partly to clearly separate framework from application as well as being able to log before app is fully setup. By request others have asked to further remove framework logging which is why it will be disabled by default in 3.2, and why the project template disables it by default now.
I actually mention in this deprecation the idea of using app.log once it is available.
|
Please find my comments above. Everything alse covers all my requirements, thanks for your rework and adding that to Cement. |
|
@derks Please check my review, this is an important change. You won't add an attachment including the path of the file from where it comes as attachment name! |
|
Oof... good catch. Thank you I'll fix that. |
cement/ext/ext_smtp.py
Outdated
| attname = in_path[0] | ||
| path = in_path[1] | ||
| else: | ||
| attname = in_path |
There was a problem hiding this comment.
attname by default should not contain a path for attachment disposition
attname = os.path.basename(in_path)There was a problem hiding this comment.
Not sure if you export the basename() function somewhere in cement like fs?
|
Hey @derks I add some comments again, hopefully you like this kind of conversation. Otherwise give me a hint to stop. |
SMTP Extension Bug Fixes and Enhancements