Repository navigation
Conversation
The CLI passed it to validate_email() as default_timeout, a keyword the function does not have, so setting it raised TypeError for every address. Hand the value to the DNS resolver that actually uses it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Setting
DEFAULT_TIMEOUT, whichemail_validator/__main__.py's own header comment documents as the way to setvalidate_emailkeyword arguments from the environment, stops the CLI from working at all:DEFAULT_TIMEOUT=5 python -m email_validator test@example.comraisesTypeError: validate_email() got an unexpected keyword argument 'default_timeout', once per address and on the STDIN path too.The loop wrote
options['default_timeout'], but the DNS timeout keyword ofvalidate_email()istimeout(validate_email.py:29). Passingtimeoutis only half of it:validate_email()refuses atimeouttogether with adns_resolver(deliverability.py:43-44raisesValueError), and the STDIN path always has a resolver because the CLI builds one for the whole file. So the timeout now goes tocaching_resolver()on that path, and is dropped when the caller injected a resolver that already carries its own lifetime.Test:
.venv/bin/python -m pytest tests/test_main.py -k default_timeoutfails ondcc3218withTypeError: validate_email() got an unexpected keyword argument 'default_timeout'(3 failed, 14 deselected) and passes here (3 passed). The repo's own gates:make lintandmake typingare clean, andmake testis 1 failed, 322 passed, 1 deselected — that failure istests/test_syntax.py::test_domain_literal, which fails on basedcc3218with the same assertion ('[IPv6:::ffff:102:304]' != '[IPv6:::ffff:1.2.3.4]') in code this PR does not touch.Details
Whether the value lands where it is supposed to: with
DEFAULT_TIMEOUT=3and no injected resolver,dns.resolver.get_default_resolver().lifetimeis3.0after the CLI runs, versus15when the variable is unset. The path isvalidate_email.py:59-60then:199-201thendeliverability.py:37-42.Two of the three new tests pass the test resolver, so they cover "the CLI no longer crashes". The third patches
caching_resolveronly to assert the number it is handed, because that STDIN branch is the one place where the CLI creates the resolver itself.DEFAULT_TIMEOUTstill breaks the naming rule stated in that header comment ("environment variables of the same name but uppercase", which would beTIMEOUT). I kept the documented name and only wired it to the right keyword, since that is the smaller change and no working behaviour depends on the current name. Say the word if you would rather it beTIMEOUTand I will do it in one commit.Deliberately not in this PR: the loop above does
options[varname.lower()] = bool(os.environ[varname]), soCHECK_DELIVERABILITY=false,0andnoall turn deliverability checking on, and only an empty string turns it off. That is a different root cause and it needs a decision on which spellings mean false, so I left it for you rather than inventing a convention here.