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

Explicitly export cron symbols for typecheckers #3072

Merged
merged 2 commits into from
Jun 12, 2024

Conversation

spladug
Copy link
Contributor

@spladug spladug commented May 14, 2024

Mypy with no_implicit_reexport = true does not see the symbols in sentry_sdk.crons as exported.

my_file.py:10: error: Module "sentry_sdk.crons" does not explicitly export attribute "monitor"  [attr-defined]

Adding the X as X to the imports marks the symbol as exported and silences the error.

An alternative approach would be to list the symbols in __all__. I'm happy to rework this if you'd prefer that.

https://mypy.readthedocs.io/en/stable/command_line.html#cmdoption-mypy-no-implicit-reexport

I've not audited other parts of the library for this; this just fixes the one module that I'm hitting issues with.

@antonpirker antonpirker added Component: DX Dealing with developer experience Component: SDK Core Dealing with the core of the SDK Component: Tests Dealing with tests labels Jun 7, 2024
@antonpirker antonpirker added this to the SDK Core milestone Jun 7, 2024
@sentrivana
Copy link
Contributor

Hey @spladug, thanks for this PR! I'd prefer we go with __all__ here -- are you still interested in reworking this?

@spladug
Copy link
Contributor Author

spladug commented Jun 11, 2024

Sure thing!

@spladug spladug force-pushed the reexport-cron-symbols branch from e4706c5 to 88d4259 Compare June 11, 2024 15:57
@spladug
Copy link
Contributor Author

spladug commented Jun 11, 2024

Rebased and updated to use __all__ instead, @sentrivana!

@sentrivana sentrivana added the Trigger: tests using secrets PR code is safe; run CI label Jun 12, 2024
@sentrivana sentrivana merged commit e5e2016 into getsentry:master Jun 12, 2024
112 of 114 checks passed
@sentrivana
Copy link
Contributor

Thanks @spladug, merged. :)

arjennienhuis pushed a commit to arjennienhuis/sentry-python that referenced this pull request Sep 30, 2024
Mypy with no_implicit_reexport = true does not see the symbols in sentry_sdk.crons as exported:

my_file.py:10: error: Module "sentry_sdk.crons" does not explicitly export attribute "monitor"  [attr-defined]

Adding the symbols to __all__ marks them as exported and silences the error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: DX Dealing with developer experience Component: SDK Core Dealing with the core of the SDK Component: Tests Dealing with tests Trigger: tests using secrets PR code is safe; run CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants