Skip to content

Conversation

Aearsears
Copy link

@Aearsears Aearsears commented Aug 19, 2025

Type of Changes

Type
βœ“ πŸ› Bug fix
✨ New feature
πŸ”¨ Refactoring
πŸ“œ Docs

Description

This PR updates the StdLibchecker to check for invalid-envvar-default and invalid-envvar-value when calling os.environ.get. os.environ is a mapping object and it calls concretely builtins.dict 's get method

Relates to PR #10094

Closes #10092

@Aearsears
Copy link
Author

should I create a news fragment for this change?

@Pierre-Sassoulas
Copy link
Member

Yes, indeed :) thank you for the MR, I skimmed and i've seen nothing shocking, will review in depth later. Great first contribution !

Copy link
Contributor

πŸ€– Effect of this PR on checked open source code: πŸ€–

Effect on home-assistant:
The following messages are now emitted:

  1. invalid-envvar-default:
    os.environ.get default type is builtins.int. Expected str or None.
    https://github.com/home-assistant/core/blob/15505cdd56f860e21155474f8bd92c044411cb69/homeassistant/util/resource.py#L20

Effect on black:
The following messages are now emitted:

  1. invalid-envvar-default:
    os.environ.get default type is builtins.int. Expected str or None.
    https://github.com/psf/black/blob/abd5f08107b09f768b1ec98c7a48e65ec7fdd661/src/black/concurrency.py#L86

Effect on django:
The following messages are now emitted:

  1. invalid-envvar-default:
    os.environ.get default type is builtins.int. Expected str or None.
    https://github.com/django/django/blob/ad4a9e0f3b1de261409bc083aa49dba705531824/django/utils/autoreload.py#L434

Effect on sentry:
The following messages are now emitted:

  1. invalid-envvar-default:
    os.environ.get default type is builtins.bool. Expected str or None.
    https://github.com/getsentry/sentry/blob/9ed4ed8c78e6163eb443023a05ef37b3739cb82a/src/sentry/conf/server.py#L735
  2. invalid-envvar-default:
    os.environ.get default type is builtins.bool. Expected str or None.
    https://github.com/getsentry/sentry/blob/9ed4ed8c78e6163eb443023a05ef37b3739cb82a/src/sentry/conf/server.py#L1971
  3. invalid-envvar-default:
    os.environ.get default type is builtins.bool. Expected str or None.
    https://github.com/getsentry/sentry/blob/9ed4ed8c78e6163eb443023a05ef37b3739cb82a/src/sentry/conf/server.py#L2983
  4. invalid-envvar-default:
    os.environ.get default type is builtins.bool. Expected str or None.
    https://github.com/getsentry/sentry/blob/9ed4ed8c78e6163eb443023a05ef37b3739cb82a/src/sentry/conf/server.py#L3023
  5. invalid-envvar-default:
    os.environ.get default type is builtins.bool. Expected str or None.
    https://github.com/getsentry/sentry/blob/9ed4ed8c78e6163eb443023a05ef37b3739cb82a/src/sentry/conf/server.py#L3140
  6. invalid-envvar-default:
    os.environ.get default type is builtins.bool. Expected str or None.
    https://github.com/getsentry/sentry/blob/9ed4ed8c78e6163eb443023a05ef37b3739cb82a/src/sentry/conf/server.py#L3885
  7. invalid-envvar-default:
    os.environ.get default type is builtins.bool. Expected str or None.
    https://github.com/getsentry/sentry/blob/9ed4ed8c78e6163eb443023a05ef37b3739cb82a/src/sentry/apidocs/utils.py#L74
  8. invalid-envvar-default:
    os.environ.get default type is builtins.bool. Expected str or None.
    https://github.com/getsentry/sentry/blob/9ed4ed8c78e6163eb443023a05ef37b3739cb82a/src/sentry/runner/settings.py#L129
  9. invalid-envvar-default:
    os.environ.get default type is builtins.int. Expected str or None.
    https://github.com/getsentry/sentry/blob/9ed4ed8c78e6163eb443023a05ef37b3739cb82a/src/sentry/testutils/pytest/sentry.py#L403
  10. invalid-envvar-default:
    os.environ.get default type is builtins.int. Expected str or None.
    https://github.com/getsentry/sentry/blob/9ed4ed8c78e6163eb443023a05ef37b3739cb82a/src/sentry/testutils/pytest/sentry.py#L404

This comment was generated for commit 5f36561

@Pierre-Sassoulas
Copy link
Member

Seeing the messages in the primer, we should probably not raise for int(os.get.environ("SOMETHING", 1) or float(os.get.environ("SOMETHING", 1.0), but let's remember that it's the work of a type checker to warn about this not pylint, and type checkers exists. (Don't want to discourage you, and you did great work, but they are more value in fixing other checks and other bugs in the backlog)

@Aearsears
Copy link
Author

Aearsears commented Aug 23, 2025

thanks for the review Pierre-Sassoulas, i'll see if I can find a quick solution to what you mentioned, otherwise i'll close this pr and contribute to other issues as you suggested

@Aearsears Aearsears force-pushed the flag-os-environ-get branch from 5f36561 to 1b65560 Compare August 23, 2025 18:49
@Aearsears Aearsears marked this pull request as ready for review August 23, 2025 22:12
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.

Question: Is there a reason that invalid-envvar-default only checks os.getenv, not os.environ.get
2 participants