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

sentry_sdk.init fails mypy checks #1415

Closed
dhuckins opened this issue Apr 28, 2022 · 10 comments · Fixed by #1421
Closed

sentry_sdk.init fails mypy checks #1415

dhuckins opened this issue Apr 28, 2022 · 10 comments · Fixed by #1421

Comments

@dhuckins
Copy link

How do you use Sentry?

Sentry Saas (sentry.io)

Version

1.5.10

Steps to Reproduce

  1. $ python3.10 -m venv venv
  2. $ source ./venv/bin/activate # assuming unix
  3. [venv] $ pip install sentry-sdk==1.5.10 mypy==0.950 (currently the latest versions of sentry-sdk and mypy)
  4. create file to test (mytest.py for example purposes)
# mytest.py
import sentry_sdk

sentry_sdk.init(dsn="this is a string")
  1. [venv] $ mypy mytest.py

Expected Result

no errors

Actual Result

❯ mypy mytest.py                                        
mytest.py:3: error: Cannot instantiate abstract class "init" with abstract attribute "__exit__"
Found 1 error in 1 file (checked 1 source file)
@vladanpaunovic
Copy link
Contributor

Thanks @dhuckins , we are on this and we expect to have a fix for this by Wednesday.

@dhuckins
Copy link
Author

Thanks @dhuckins , we are on this and we expect to have a fix for this by Wednesday.

awesome! thank you!

@dhuckins
Copy link
Author

dhuckins commented May 3, 2022

thanks for taking care of this so quickly! when is the next release (with these changes) planned?

@sl0thentr0py
Copy link
Member

@dhuckins released 1.5.11

@dhuckins
Copy link
Author

dhuckins commented May 3, 2022

Thanks!

@takeda
Copy link

takeda commented May 3, 2022

@sl0thentr0py any reason why https://peps.python.org/pep-0612 or python/typing#270 (comment) was not used as a fix? I was looking at python/typing#270 and saw that @untitaker was involved in discussion about it so I assumed that it would be the fix.

@untitaker
Copy link
Member

Honestly I can't wrap my head around that PEP right now so if you think you have a better solution i'd create a patch.

@takeda
Copy link

takeda commented May 4, 2022

@untitaker I took a closer I guess PEP 612 does not apply.

How I understand that could work, is to remove the hacks with _Client and Client make the class simply being called Client and _get_options and get_options then copy all of the parameters from the mockup Client to get_options (alternatively use TypedDict).

then add:

F = TypeVar('F', bound=Callable[..., Any])

class copy_signature(Generic[F]):
    def __init__(self, target: F) -> None: ...
    def __call__(self, wrapped: Callable[..., Any]) -> F: ...

As mentioned in the comment, and add then @copy_signature(get_options) to the init() function.

@sl0thentr0py
Copy link
Member

sl0thentr0py commented May 4, 2022

@takeda if you make a PR, I'll be happy to merge in the changes

@untitaker
Copy link
Member

I think using a decorator instead of a class hierarchy to patch in the signature might be a good idea that makes things more legible, but I think TypedDict (still) cannot be used to type out kwargs.

tbrlpld added a commit to tbrlpld/lpld-io that referenced this issue Mar 3, 2023
Error:
lpld/settings.py:398: error: Cannot instantiate abstract class "init" with abstract attribute "__exit__"  [abstract]

See also: getsentry/sentry-python#1415
tbrlpld added a commit to tbrlpld/lpld-io that referenced this issue Mar 3, 2023
Error:
lpld/settings.py:398: error: Cannot instantiate abstract class "init" with abstract attribute "__exit__"  [abstract]

See also: getsentry/sentry-python#1415
tbrlpld added a commit to tbrlpld/lpld-io that referenced this issue Mar 3, 2023
Error:
lpld/settings.py:398: error: Cannot instantiate abstract class "init" with abstract attribute "__exit__"  [abstract]

See also: getsentry/sentry-python#1415
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 a pull request may close this issue.

6 participants