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

Backport update to email-validator>=2.0.0.post2 #5627

Merged
merged 11 commits into from Apr 30, 2023

Conversation

adriangb
Copy link
Member

@adriangb adriangb commented Apr 28, 2023

Fixes #3905

Selected Reviewer: @samuelcolvin

@adriangb adriangb self-assigned this Apr 28, 2023
@adriangb
Copy link
Member Author

@tiangolo looks like we're blocked by this: https://github.com/tiangolo/fastapi/blob/8ac8d70d52bb0dd9eb55ba4e22d3e383943da05c/pyproject.toml#L62

Any chance you could cut a FastAPI patch release that removes the upper version pin?

@cloudflare-pages
Copy link

cloudflare-pages bot commented Apr 29, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: b593e2a
Status:🚫  Build failed.

View logs

Comment on lines +733 to +739
else:
# email-validator >1, <2
at_index = email.index('@')
local_part = email[:at_index] # RFC 5321, local part must be case-sensitive.
global_part = email[at_index:].lower()

return name or local_part, local_part + global_part
Copy link
Member Author

@adriangb adriangb Apr 29, 2023

Choose a reason for hiding this comment

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

This is the exact same code we had previously. To avoid adding a whole CI run just to test this version of email-validator, we are going to leave it uncovered. I ran locally and all tests pass with email-validator <2. This is also being run via the FastAPI tests, it's just not counted towards coverage.

@adriangb
Copy link
Member Author

please review

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

otherwise LGTM.

Please update, or merge if you're confident this is right.

setup.py Outdated Show resolved Hide resolved
@adriangb adriangb merged commit 8131196 into 1.10.X-fixes Apr 30, 2023
49 of 50 checks passed
@adriangb adriangb deleted the update-email-validator branch April 30, 2023 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants