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

ref(typing): Add additional stub packages for type checking #3122

Merged
merged 11 commits into from
Jun 25, 2024
Merged
1 change: 1 addition & 0 deletions requirements-docs.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
gevent
shibuya
sphinx==7.2.6
sphinx-autodoc-typehints[type_comments]>=1.8.0
Expand Down
3 changes: 3 additions & 0 deletions requirements-linting.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ black
flake8==5.0.4 # flake8 depends on pyflakes>=3.0.0 and this dropped support for Python 2 "# type:" comments
types-certifi
types-protobuf
types-gevent
types-greenlet
types-redis
types-setuptools
types-webob
pymongo # There is no separate types module.
loguru # There is no separate types module.
flake8-bugbear
Expand Down
3 changes: 2 additions & 1 deletion sentry_sdk/integrations/_wsgi_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from typing import Any
from typing import Dict
from typing import Mapping
from typing import MutableMapping
from typing import Optional
from typing import Union
from sentry_sdk._types import Event, HttpStatusCodeRange
Expand Down Expand Up @@ -114,7 +115,7 @@ def content_length(self):
return 0

def cookies(self):
# type: () -> Dict[str, Any]
# type: () -> MutableMapping[str, Any]
raise NotImplementedError()

def raw_data(self):
Expand Down
8 changes: 4 additions & 4 deletions sentry_sdk/integrations/pyramid.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
from typing import Callable
from typing import Dict
from typing import Optional
from webob.cookies import RequestCookies # type: ignore
from webob.compat import cgi_FieldStorage # type: ignore
from webob.cookies import RequestCookies
from webob.request import _FieldStorageWithFile

from sentry_sdk.utils import ExcInfo
from sentry_sdk._types import Event, EventProcessor
Expand Down Expand Up @@ -189,15 +189,15 @@ def form(self):
}

def files(self):
# type: () -> Dict[str, cgi_FieldStorage]
# type: () -> Dict[str, _FieldStorageWithFile]
return {
key: value
for key, value in self.request.POST.items()
if getattr(value, "filename", None)
}

def size_of_file(self, postdata):
# type: (cgi_FieldStorage) -> int
# type: (_FieldStorageWithFile) -> int
file = postdata.file
try:
return os.fstat(file.fileno()).st_size
Expand Down
10 changes: 6 additions & 4 deletions sentry_sdk/profiler/continuous_profiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from typing import Dict
from typing import List
from typing import Optional
from typing import Type
from typing import Union
from typing_extensions import TypedDict
from sentry_sdk._types import ContinuousProfilerMode
Expand All @@ -51,9 +52,10 @@


try:
from gevent.monkey import get_original # type: ignore
from gevent.threadpool import ThreadPool # type: ignore
from gevent.monkey import get_original
from gevent.threadpool import ThreadPool as _ThreadPool

ThreadPool = _ThreadPool # type: Optional[Type[_ThreadPool]]
Comment on lines +56 to +58
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary?

Copy link
Contributor Author

@Daverball Daverball Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't do this, then mypy will complain about redefinition of ThreadPool to None in the except clause. So the type and the variable need a separate name, so I can use the type separately from the runtime variable.

thread_sleep = get_original("time", "sleep")
except ImportError:
thread_sleep = time.sleep
Expand Down Expand Up @@ -347,7 +349,7 @@ def __init__(self, frequency, options, capture_func):

super().__init__(frequency, options, capture_func)

self.thread = None # type: Optional[ThreadPool]
self.thread = None # type: Optional[_ThreadPool]
self.pid = None # type: Optional[int]
self.lock = threading.Lock()

Expand Down Expand Up @@ -377,7 +379,7 @@ def ensure_running(self):
# we should create a new buffer along with it
self.reset_buffer()

self.thread = ThreadPool(1)
self.thread = ThreadPool(1) # type: ignore[misc]
try:
self.thread.spawn(self.run)
except RuntimeError:
Expand Down
10 changes: 6 additions & 4 deletions sentry_sdk/profiler/transaction_profiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
from typing import List
from typing import Optional
from typing import Set
from typing import Type
from typing_extensions import TypedDict

from sentry_sdk.profiler.utils import (
Expand Down Expand Up @@ -95,9 +96,10 @@


try:
from gevent.monkey import get_original # type: ignore
from gevent.threadpool import ThreadPool # type: ignore
from gevent.monkey import get_original
from gevent.threadpool import ThreadPool as _ThreadPool

ThreadPool = _ThreadPool # type: Optional[Type[_ThreadPool]]
thread_sleep = get_original("time", "sleep")
except ImportError:
thread_sleep = time.sleep
Expand Down Expand Up @@ -738,7 +740,7 @@ def __init__(self, frequency):

# used to signal to the thread that it should stop
self.running = False
self.thread = None # type: Optional[ThreadPool]
self.thread = None # type: Optional[_ThreadPool]
self.pid = None # type: Optional[int]

# This intentionally uses the gevent patched threading.Lock.
Expand Down Expand Up @@ -775,7 +777,7 @@ def ensure_running(self):
self.pid = pid
self.running = True

self.thread = ThreadPool(1)
self.thread = ThreadPool(1) # type: ignore[misc]
try:
self.thread.spawn(self.run)
except RuntimeError:
Expand Down
18 changes: 11 additions & 7 deletions sentry_sdk/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@
Union,
)

from gevent.hub import Hub
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since gevent is an optional dependency, won't this break type checking for folks that don't have it installed? Would a conditional import help in any way?

Copy link
Contributor Author

@Daverball Daverball Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no conditional imports for typing unfortunately. So yes, if people leave follow_imports at normal for sentry_sdk they will see an error if they don't have types-gevent installed, but the same is true for any of the other typing only dependencies that are only listed in dev-requirements.txt, so this would be nothing new.

Generally the expectation with type hints is that people are responsible for managing typing- requirements and otherwise silence the errors by setting follow_imports to silent or skip.


import sentry_sdk.integrations
from sentry_sdk._types import Event, ExcInfo

Expand Down Expand Up @@ -1182,8 +1184,8 @@ def _is_contextvars_broken():
Returns whether gevent/eventlet have patched the stdlib in a way where thread locals are now more "correct" than contextvars.
"""
try:
import gevent # type: ignore
from gevent.monkey import is_object_patched # type: ignore
import gevent
from gevent.monkey import is_object_patched

# Get the MAJOR and MINOR version numbers of Gevent
version_tuple = tuple(
Expand All @@ -1209,7 +1211,7 @@ def _is_contextvars_broken():
pass

try:
import greenlet # type: ignore
import greenlet
from eventlet.patcher import is_monkey_patched # type: ignore

greenlet_version = parse_version(greenlet.__version__)
Expand Down Expand Up @@ -1794,12 +1796,14 @@ def now():
from gevent.monkey import is_module_patched
except ImportError:

def get_gevent_hub():
# type: () -> Any
# it's not great that the signatures are different, get_hub can't return None
# consider adding an if TYPE_CHECKING to change the signature to Optional[Hub]
def get_gevent_hub(): # type: ignore[misc]
# type: () -> Optional[Hub]
return None

def is_module_patched(*args, **kwargs):
# type: (*Any, **Any) -> bool
def is_module_patched(mod_name):
# type: (str) -> bool
# unable to import from gevent means no modules have been patched
return False

Expand Down
Loading