-
-
Notifications
You must be signed in to change notification settings - Fork 195
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
Respect new strict parsing of email.utils.getaddresses
#4023
Conversation
@petschki thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment:
To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically. Happy hacking! |
4ed7b60
to
4504573
Compare
@jenkins-plone-org please run jobs |
4504573
to
ad95869
Compare
@jenkins-plone-org please run jobs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this. I have a few suggestions.
PloneTool.py
has these methods:
validateEmailAddresses
: validate a string with possibly multiple email addresses. For each address this callsvalidateSingleNormalizedEmailAddress
.validateSingleEmailAddress
: validate a string with at most ONE email address. For this address this callsvalidateSingleNormalizedEmailAddress
.validateSingleNormalizedEmailAddress
only needs to handle one address, obviously.
- In all three methods: after checking if it is a string, can we do a
.strip()
? - In the two methods handling only a single address, can we do
address.splitlines()
and exit when this returns more than one line? In both cases this can replace the use ofEMAIL_CUTOFF_RE
.
BTW, I agree with your removing this line from the valid inputs:
user@example.org\n user2@example.org", # omitting comma is ok
I first moved this line to the invalid inputs but then it fails on current py3.10 jenkins with no strict parsing. |
@jenkins-plone-org please run jobs |
Currently its hard to get a green jenkins ... |
@jenkins-plone-org please run jobs |
@jenkins-plone-org please run jobs |
1 similar comment
@jenkins-plone-org please run jobs |
Is Node4 experiencing flakiness again, or could the issues be related to code changes? |
Ah, I understood... thanks for providing more info! |
Currently the build is broken because of this strict parsing which got backported to 3.10 too (see https://jenkins.plone.org/job/plone-6.1-python-3.10/371/) ... anyone willing to approve here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for working on this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I assume the 6.1-3.12 jenkins job will fail due to the flaky robottests ... if the 3.10 job is green I'll merge. EDIT: I do not have permissions to merge |
preferred solution for #4020 ... successor of #4021