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.

Sign up for GitHub

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

Running mypy on sdk resources #773 #4360

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Running mypy on sdk resources #773 #4360

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3ab4dc7
Running mypy on sdk resources #773
prabhakarjuzgar Dec 16, 2024
c38154e
Update changelog
prabhakarjuzgar Dec 16, 2024
5c6d89b
Remove unicode chars and unwanted ignore
prabhakarjuzgar Dec 18, 2024
fc6c6e5
Remove unnecessary changes from CHANGELOG
prabhakarjuzgar Dec 18, 2024
822e841
Change importlib_metadata to importlib.metadata
prabhakarjuzgar Dec 20, 2024
192db0c
Address review comment
prabhakarjuzgar Dec 20, 2024
853c00f
Use importlib_metadata if importlib.data is unavailable
prabhakarjuzgar Dec 21, 2024
2ddbd55
Merge branch 'main' into pr773
xrmx Dec 23, 2024
d42a650
Remove unnecessary changes
prabhakarjuzgar Dec 23, 2024
99f1ff4
do not change implementation
prabhakarjuzgar Dec 23, 2024
825c727
do not change implementation final
prabhakarjuzgar Dec 23, 2024
ce4c3a7
suppress specific warnings
prabhakarjuzgar Dec 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

- Add type hints to SDK resources
([#4360](https://github.com/open-telemetry/opentelemetry-python/pull/4360))
- Add `attributes` field in `metrics.get_meter` wrapper function
([#4364](https://github.com/open-telemetry/opentelemetry-python/pull/4364))
- Add Python 3.13 support
Expand Down Expand Up @@ -279,7 +281,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## Version 1.21.0/0.42b0 (2023-11-01)

- Fix `SumAggregation`
 ([#3390](https://github.com/open-telemetry/opentelemetry-python/pull/3390))
([#3390](https://github.com/open-telemetry/opentelemetry-python/pull/3390))
- Fix handling of empty metric collection cycles
([#3335](https://github.com/open-telemetry/opentelemetry-python/pull/3335))
- Fix error when no LoggerProvider configured for LoggingHandler
Expand All @@ -299,7 +301,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Implement Process Resource detector
([#3472](https://github.com/open-telemetry/opentelemetry-python/pull/3472))


## Version 1.20.0/0.41b0 (2023-09-04)

- Modify Prometheus exporter to translate non-monotonic Sums into Gauges
Expand Down Expand Up @@ -328,7 +329,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Default LogRecord observed_timestamp to current timestamp
[#3377](https://github.com/open-telemetry/opentelemetry-python/pull/3377))


## Version 1.18.0/0.39b0 (2023-05-19)

- Select histogram aggregation with an environment variable
Expand All @@ -348,7 +348,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Add benchmark tests for metrics
([#3267](https://github.com/open-telemetry/opentelemetry-python/pull/3267))


## Version 1.17.0/0.38b0 (2023-03-22)

- Implement LowMemory temporality
Expand Down Expand Up @@ -1696,7 +1695,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Remove dependency on 'backoff' library
([#3679](https://github.com/open-telemetry/opentelemetry-python/pull/3679))


- Make create_gauge non-abstract method
([#3817](https://github.com/open-telemetry/opentelemetry-python/pull/3817))
- Make `tracer.start_as_current_span()` decorator work with async functions
Expand Down
27 changes: 14 additions & 13 deletions opentelemetry-sdk/src/opentelemetry/sdk/error_handler/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ def _handle(self, error: Exception, *args, **kwargs):

from abc import ABC, abstractmethod
from logging import getLogger
from typing import Optional
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather prefer to use from __future__ import annotations on the top of the imports, and use pipe (|) annotation.


from opentelemetry.util._importlib_metadata import entry_points

Expand All @@ -69,7 +70,7 @@ def _handle(self, error: Exception, *args, **kwargs):

class ErrorHandler(ABC):
@abstractmethod
def _handle(self, error: Exception, *args, **kwargs):
def _handle(self, error: Exception, *args, **kwargs) -> None: # type: ignore[misc, no-untyped-def]
Copy link
Contributor

Choose a reason for hiding this comment

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

The no-untyped-def can be removed by properly type hint it:

Suggested change
def _handle(self, error: Exception, *args, **kwargs) -> None: # type: ignore[misc, no-untyped-def]
def _handle(self, error: Exception, *args: Any, **kwargs: Any) -> None:

Why is the misc there?

"""
Handle an exception
"""
Expand All @@ -83,7 +84,7 @@ class _DefaultErrorHandler(ErrorHandler):
"""

# pylint: disable=useless-return
def _handle(self, error: Exception, *args, **kwargs):
def _handle(self, error: Exception, *args, **kwargs) -> None: # type: ignore[no-untyped-def]
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

logger.exception("Error handled by default error handler: ")
return None

Expand All @@ -105,26 +106,26 @@ def __new__(cls) -> "GlobalErrorHandler":

return cls._instance

def __enter__(self):
def __enter__(self) -> None:
pass

# pylint: disable=no-self-use
def __exit__(self, exc_type, exc_value, traceback):
if exc_value is None:
def __exit__(self, exc_type, exc_value, traceback) -> Optional[bool]: # type: ignore[no-untyped-def]
Copy link
Contributor

Choose a reason for hiding this comment

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

Use type hints mentioned here: https://stackoverflow.com/a/58808179

And remove the type ignore.

if exc_value is None: # type: ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

If above applied, the ignore is not needed.

Suggested change
if exc_value is None: # type: ignore
if exc_value is None:

return None

plugin_handled = False

error_handler_entry_points = entry_points(
error_handler_entry_points = entry_points( # type: ignore[misc]
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the misc error here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Same question applies to the below.

group="opentelemetry_error_handler"
)

for error_handler_entry_point in error_handler_entry_points:
error_handler_class = error_handler_entry_point.load()
for error_handler_entry_point in error_handler_entry_points: # type: ignore[misc]
error_handler_class = error_handler_entry_point.load() # type: ignore[misc]

if issubclass(error_handler_class, exc_value.__class__):
if issubclass(error_handler_class, exc_value.__class__): # type: ignore[misc]
try:
error_handler_class()._handle(exc_value)
error_handler_class()._handle(exc_value) # type: ignore[misc]
plugin_handled = True

# pylint: disable=broad-exception-caught
Expand All @@ -133,11 +134,11 @@ def __exit__(self, exc_type, exc_value, traceback):
"%s error while handling error"
" %s by error handler %s",
error_handling_error.__class__.__name__,
exc_value.__class__.__name__,
error_handler_class.__name__,
exc_value.__class__.__name__, # type: ignore[misc]
error_handler_class.__name__, # type: ignore[misc]
)

if not plugin_handled:
_DefaultErrorHandler()._handle(exc_value)
_DefaultErrorHandler()._handle(exc_value) # type: ignore[misc]

return True
14 changes: 7 additions & 7 deletions opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar_reservoir.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def collect(self, point_attributes: Attributes) -> Optional[Exemplar]:
{
k: v
for k, v in self.__attributes.items()
if k not in point_attributes
if k not in point_attributes # type: ignore[operator]
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems valid. What values Attributes can assume?

}
if self.__attributes
else None
Expand Down Expand Up @@ -162,8 +162,8 @@ class BucketIndexError(ValueError):
class FixedSizeExemplarReservoirABC(ExemplarReservoir):
"""Abstract class for a reservoir with fixed size."""

def __init__(self, size: int, **kwargs) -> None:
super().__init__(**kwargs)
def __init__(self, size: int, **kwargs) -> None: # type: ignore[no-untyped-def]
super().__init__(**kwargs) # type: ignore[misc]
Comment on lines +165 to +166
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the signature of ExemplarReservoir?

self._size: int = size
self._reservoir_storage: Mapping[int, ExemplarBucket] = defaultdict(
ExemplarBucket
Expand Down Expand Up @@ -257,8 +257,8 @@ class SimpleFixedSizeExemplarReservoir(FixedSizeExemplarReservoirABC):
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#simplefixedsizeexemplarreservoir
"""

def __init__(self, size: int = 1, **kwargs) -> None:
super().__init__(size, **kwargs)
def __init__(self, size: int = 1, **kwargs) -> None: # type: ignore[no-untyped-def]
super().__init__(size, **kwargs) # type: ignore[misc]
self._measurements_seen: int = 0

def _reset(self) -> None:
Expand Down Expand Up @@ -292,8 +292,8 @@ class AlignedHistogramBucketExemplarReservoir(FixedSizeExemplarReservoirABC):
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#alignedhistogrambucketexemplarreservoir
"""

def __init__(self, boundaries: Sequence[float], **kwargs) -> None:
super().__init__(len(boundaries) + 1, **kwargs)
def __init__(self, boundaries: Sequence[float], **kwargs) -> None: # type: ignore[no-untyped-def]
super().__init__(len(boundaries) + 1, **kwargs) # type: ignore[misc]
self._boundaries: Sequence[float] = boundaries

def offer(
Expand Down
Loading