From 20cf4cb087371c1e255e9efb7072a78fdad55333 Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Mon, 8 Jun 2020 12:39:57 -0400 Subject: [PATCH] refactor: Typing fixes #768, upgrade mypy to 0.770 (#769) --- dev-requirements.txt | 2 +- .../src/opentelemetry/__init__.pyi | 0 .../opentelemetry/configuration/__init__.py | 73 ++++++++++--------- .../src/opentelemetry/metrics/__init__.py | 4 +- .../src/opentelemetry/trace/__init__.py | 4 +- .../src/opentelemetry/util/__init__.py | 36 ++++++--- .../tests/configuration/test_configuration.py | 27 ++++--- 7 files changed, 83 insertions(+), 63 deletions(-) create mode 100644 opentelemetry-api/src/opentelemetry/__init__.pyi diff --git a/dev-requirements.txt b/dev-requirements.txt index be74d804b3d..253c0895072 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -2,7 +2,7 @@ pylint==2.4.4 flake8==3.7.9 isort~=4.3 black>=19.3b0,==19.* -mypy==0.740 +mypy==0.770 sphinx~=2.1 sphinx-rtd-theme~=0.4 sphinx-autodoc-typehints~=1.10.2 diff --git a/opentelemetry-api/src/opentelemetry/__init__.pyi b/opentelemetry-api/src/opentelemetry/__init__.pyi new file mode 100644 index 00000000000..e69de29bb2d diff --git a/opentelemetry-api/src/opentelemetry/configuration/__init__.py b/opentelemetry-api/src/opentelemetry/configuration/__init__.py index 48093415020..0bd2bce8d64 100644 --- a/opentelemetry-api/src/opentelemetry/configuration/__init__.py +++ b/opentelemetry-api/src/opentelemetry/configuration/__init__.py @@ -12,9 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -# FIXME find a better way to avoid all those "Expression has type "Any"" errors -# type: ignore - """ Simple configuration manager @@ -95,17 +92,23 @@ from os import environ from re import fullmatch +from typing import ClassVar, Dict, Optional, TypeVar, Union +ConfigValue = Union[str, bool, int, float] +_T = TypeVar("_T", ConfigValue, Optional[ConfigValue]) -class Configuration: - _instance = None - __slots__ = [] +class Configuration: + _instance = None # type: ClassVar[Optional[Configuration]] + _config_map = {} # type: ClassVar[Dict[str, ConfigValue]] def __new__(cls) -> "Configuration": - if Configuration._instance is None: + if cls._instance is not None: + instance = cls._instance + else: - for key, value in environ.items(): + instance = super().__new__(cls) + for key, value_str in environ.items(): match = fullmatch( r"OPENTELEMETRY_PYTHON_([A-Za-z_][\w_]*)", key @@ -114,45 +117,47 @@ def __new__(cls) -> "Configuration": if match is not None: key = match.group(1) + value = value_str # type: ConfigValue - if value == "True": + if value_str == "True": value = True - elif value == "False": + elif value_str == "False": value = False else: try: - value = int(value) + value = int(value_str) except ValueError: pass try: - value = float(value) + value = float(value_str) except ValueError: pass - setattr(Configuration, "_{}".format(key), value) - setattr( - Configuration, - key, - property( - fget=lambda cls, key=key: getattr( - cls, "_{}".format(key) - ) - ), - ) + instance._config_map[key] = value - Configuration.__slots__.append(key) + cls._instance = instance - Configuration.__slots__ = tuple(Configuration.__slots__) + return instance - Configuration._instance = object.__new__(cls) + def __getattr__(self, name: str) -> Optional[ConfigValue]: + return self._config_map.get(name) - return cls._instance + def __setattr__(self, key: str, val: ConfigValue) -> None: + if key == "_config_map": + super().__setattr__(key, val) + else: + raise AttributeError(key) - def __getattr__(self, name): - return None + def get(self, name: str, default: _T) -> _T: + """Use this typed method for dynamic access instead of `getattr` + + :rtype: str or bool or int or float or None + """ + val = self._config_map.get(name, default) + return val @classmethod - def _reset(cls): + def _reset(cls) -> None: """ This method "resets" the global configuration attributes @@ -160,10 +165,6 @@ def _reset(cls): only. """ - for slot in cls.__slots__: - if slot in cls.__dict__.keys(): - delattr(cls, slot) - delattr(cls, "_{}".format(slot)) - - cls.__slots__ = [] - cls._instance = None + if cls._instance: + cls._instance._config_map.clear() # pylint: disable=protected-access + cls._instance = None diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py index 16ac4c096f0..7d16c56a98b 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -31,7 +31,7 @@ from logging import getLogger from typing import Callable, Dict, Optional, Sequence, Tuple, Type, TypeVar -from opentelemetry.util import _load_provider +from opentelemetry.util import _load_meter_provider logger = getLogger(__name__) ValueT = TypeVar("ValueT", int, float) @@ -395,6 +395,6 @@ def get_meter_provider() -> MeterProvider: global _METER_PROVIDER # pylint: disable=global-statement if _METER_PROVIDER is None: - _METER_PROVIDER = _load_provider("meter_provider") + _METER_PROVIDER = _load_meter_provider("meter_provider") return _METER_PROVIDER diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 13aaf2c6a4f..25af91a4007 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -78,7 +78,7 @@ from logging import getLogger from opentelemetry.trace.status import Status -from opentelemetry.util import _load_provider, types +from opentelemetry.util import _load_trace_provider, types logger = getLogger(__name__) @@ -706,6 +706,6 @@ def get_tracer_provider() -> TracerProvider: global _TRACER_PROVIDER # pylint: disable=global-statement if _TRACER_PROVIDER is None: - _TRACER_PROVIDER = _load_provider("tracer_provider") + _TRACER_PROVIDER = _load_trace_provider("tracer_provider") return _TRACER_PROVIDER diff --git a/opentelemetry-api/src/opentelemetry/util/__init__.py b/opentelemetry-api/src/opentelemetry/util/__init__.py index bab42b0bde2..ed1268fcb62 100644 --- a/opentelemetry-api/src/opentelemetry/util/__init__.py +++ b/opentelemetry-api/src/opentelemetry/util/__init__.py @@ -14,11 +14,17 @@ import re import time from logging import getLogger -from typing import Sequence, Union +from typing import TYPE_CHECKING, Sequence, Union, cast from pkg_resources import iter_entry_points -from opentelemetry.configuration import Configuration # type: ignore +from opentelemetry.configuration import Configuration + +if TYPE_CHECKING: + from opentelemetry.trace import TracerProvider + from opentelemetry.metrics import MeterProvider + +Provider = Union["TracerProvider", "MeterProvider"] logger = getLogger(__name__) @@ -34,25 +40,33 @@ def time_ns() -> int: return int(time.time() * 1e9) -def _load_provider( - provider: str, -) -> Union["TracerProvider", "MeterProvider"]: # type: ignore +def _load_provider(provider: str) -> Provider: try: - return next( # type: ignore + entry_point = next( iter_entry_points( "opentelemetry_{}".format(provider), - name=getattr( - Configuration(), # type: ignore - provider, - "default_{}".format(provider), + name=cast( + str, + Configuration().get( + provider, "default_{}".format(provider), + ), ), ) - ).load()() + ) + return cast(Provider, entry_point.load()(),) except Exception: # pylint: disable=broad-except logger.error("Failed to load configured provider %s", provider) raise +def _load_meter_provider(provider: str) -> "MeterProvider": + return cast("MeterProvider", _load_provider(provider)) + + +def _load_trace_provider(provider: str) -> "TracerProvider": + return cast("TracerProvider", _load_provider(provider)) + + # Pattern for matching up until the first '/' after the 'https://' part. _URL_PATTERN = r"(https?|ftp)://.*?/" diff --git a/opentelemetry-api/tests/configuration/test_configuration.py b/opentelemetry-api/tests/configuration/test_configuration.py index a817a1bf43d..32b62e619d5 100644 --- a/opentelemetry-api/tests/configuration/test_configuration.py +++ b/opentelemetry-api/tests/configuration/test_configuration.py @@ -16,16 +16,16 @@ from unittest import TestCase from unittest.mock import patch -from opentelemetry.configuration import Configuration # type: ignore +from opentelemetry.configuration import Configuration class TestConfiguration(TestCase): - def tearDown(self): + def tearDown(self) -> None: # This call resets the attributes of the Configuration class so that # each test is executed in the same conditions. Configuration._reset() - def test_singleton(self): + def test_singleton(self) -> None: self.assertIsInstance(Configuration(), Configuration) self.assertIs(Configuration(), Configuration()) @@ -39,7 +39,7 @@ def test_singleton(self): "OPENTELEMETRY_PTHON_TRACEX_PROVIDER": "tracex_provider", }, ) - def test_environment_variables(self): # type: ignore + def test_environment_variables(self): self.assertEqual( Configuration().METER_PROVIDER, "meter_provider" ) # pylint: disable=no-member @@ -62,16 +62,21 @@ def test_property(self): with self.assertRaises(AttributeError): Configuration().TRACER_PROVIDER = "new_tracer_provider" - def test_slots(self): + def test_slots(self) -> None: with self.assertRaises(AttributeError): Configuration().XYZ = "xyz" # pylint: disable=assigning-non-slot - def test_getattr(self): + def test_getattr(self) -> None: + # literal access self.assertIsNone(Configuration().XYZ) - def test_reset(self): + # dynamic access + self.assertIsNone(getattr(Configuration(), "XYZ")) + self.assertIsNone(Configuration().get("XYZ", None)) + + def test_reset(self) -> None: environ_patcher = patch.dict( - "os.environ", # type: ignore + "os.environ", {"OPENTELEMETRY_PYTHON_TRACER_PROVIDER": "tracer_provider"}, ) @@ -96,7 +101,7 @@ def test_reset(self): "OPENTELEMETRY_PYTHON_FALSE": "False", }, ) - def test_boolean(self): + def test_boolean(self) -> None: self.assertIsInstance( Configuration().TRUE, bool ) # pylint: disable=no-member @@ -114,7 +119,7 @@ def test_boolean(self): "OPENTELEMETRY_PYTHON_NON_INTEGER": "-12z3", }, ) - def test_integer(self): + def test_integer(self) -> None: self.assertEqual( Configuration().POSITIVE_INTEGER, 123 ) # pylint: disable=no-member @@ -133,7 +138,7 @@ def test_integer(self): "OPENTELEMETRY_PYTHON_NON_FLOAT": "-12z3.123", }, ) - def test_float(self): + def test_float(self) -> None: self.assertEqual( Configuration().POSITIVE_FLOAT, 123.123 ) # pylint: disable=no-member