Skip to content

Read the CLI option environment variables the way they are documented - #175

Open
feiiiiii5 wants to merge 1 commit into
JoshData:mainfrom
feiiiiii5:fix/main-env-var-options
Open

feiiiiii5 wants to merge 1 commit into
JoshData:mainfrom
feiiiiii5:fix/main-env-var-options

Conversation

@feiiiiii5

Copy link
Copy Markdown
Contributor

Two documented CLI options are unusable, and one of them is inverted:

$ CHECK_DELIVERABILITY=false python -m email_validator me@xkxufoekjvjfjeodlfmdfjcu.com
The domain name xkxufoekjvjfjeodlfmdfjcu.com does not exist.     # the check ran anyway

$ DEFAULT_TIMEOUT=5 python -m email_validator me@example.com
TypeError: validate_email() got an unexpected keyword argument 'default_timeout'

Both are in the same block of email_validator/__main__.py:

  • the boolean options go through bool(), and since every non-empty string is truthy, CHECK_DELIVERABILITY=false / TEST_ENVIRONMENT=0 / =no all mean true — the check the user asked to turn off is the one that runs, DNS lookups included;
  • DEFAULT_TIMEOUT is collected into the same options dict and forwarded as a keyword argument, but validate_email has no such parameter. The real knobs are its timeout argument and the module-level DEFAULT_TIMEOUT that caching_resolver() reads, so no value at all works.

The module's own docstring says "keyword arguments to validate_email can be set in environment variables of the same name but uppercase", the README documents these as check_deliverability=False, and the 2.0.0 changelog entry introduced the option reading — so both spellings are meant to work. The fix reads the falsy set explicitly (empty string, plus 0/false/no/off, case- and space-insensitive), which is a superset of what bool() used to reject, so nothing that works today changes, and it sets the module global for DEFAULT_TIMEOUT.

I used int() rather than float() because DEFAULT_TIMEOUT is declared as an int in __init__.py and every public timeout parameter in the library is Optional[int] — mypy rejects the float. The visible consequence is that DEFAULT_TIMEOUT=0.5 now raises ValueError instead of the old TypeError; if you would rather have fractional seconds, the consistent change is annotating DEFAULT_TIMEOUT: float and the timeout parameters, which is a bigger call than this PR should make on its own.

Test: pytest tests/test_main.py -k from_env fails on c0cc1bc — 13 of the 14 cases, with the DNS-failure text where the JSON is expected and the TypeError for the timeout — and passes on this branch. The whole suite is 331 passed / 1 deselected (that one needs real DNS). flake8 and mypy are clean.

I did not add a CHANGELOG entry under "In Development" — happy to if you want one.

Two defects in the same block, both of which make a documented option
unusable or inverted:

* the boolean options went through bool(), so CHECK_DELIVERABILITY=false,
  TEST_ENVIRONMENT=0 and any other non-empty value all meant *true* - the
  check the user asked to turn off is the one that runs, including the DNS
  lookups;
* DEFAULT_TIMEOUT was forwarded as a keyword argument, but validate_email
  has no such parameter (the real ones are timeout and the module-level
  DEFAULT_TIMEOUT), so setting it always ended in
  TypeError: validate_email() got an unexpected keyword argument
  'default_timeout'.

The module documents that "keyword arguments to validate_email can be set in
environment variables of the same name but uppercase" and 2.0.0 added the
option reading, so both spellings should mean what the docs say. The falsy
set is a superset of what bool() rejected before (the empty string, plus
0/false/no/off, case- and space-insensitive), so nothing that worked
changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant