From 41b863ab557a6e5d273384f5c91c07115ded02c1 Mon Sep 17 00:00:00 2001 From: Renan Ivo Date: Fri, 21 Jul 2023 22:17:15 -0300 Subject: [PATCH 1/6] Use importlib.metadata first to avoid deprecation warnings --- newrelic/common/package_version_utils.py | 19 ++++++++++--------- .../test_package_version_utils.py | 17 ++++++++++++++--- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/newrelic/common/package_version_utils.py b/newrelic/common/package_version_utils.py index f3d334e2a6..4582387d6e 100644 --- a/newrelic/common/package_version_utils.py +++ b/newrelic/common/package_version_utils.py @@ -70,6 +70,16 @@ def int_or_str(value): def _get_package_version(name): module = sys.modules.get(name, None) version = None + + # importlib was introduced into the standard library starting in Python3.8. + if "importlib" in sys.modules and hasattr(sys.modules["importlib"], "metadata"): + try: + version = sys.modules["importlib"].metadata.version(name) # pylint: disable=E1101 + if version not in NULL_VERSIONS: + return version + except Exception: + pass + for attr in VERSION_ATTRS: try: version = getattr(module, attr, None) @@ -84,15 +94,6 @@ def _get_package_version(name): except Exception: pass - # importlib was introduced into the standard library starting in Python3.8. - if "importlib" in sys.modules and hasattr(sys.modules["importlib"], "metadata"): - try: - version = sys.modules["importlib"].metadata.version(name) # pylint: disable=E1101 - if version not in NULL_VERSIONS: - return version - except Exception: - pass - if "pkg_resources" in sys.modules: try: version = sys.modules["pkg_resources"].get_distribution(name).version diff --git a/tests/agent_unittests/test_package_version_utils.py b/tests/agent_unittests/test_package_version_utils.py index 435d74947f..db1d2d3fb7 100644 --- a/tests/agent_unittests/test_package_version_utils.py +++ b/tests/agent_unittests/test_package_version_utils.py @@ -40,6 +40,17 @@ def patched_pytest_module(monkeypatch): yield pytest +@pytest.fixture(scope="function") +def remove_importlib(): + if "importlib" not in sys.modules: + return + + importlib = sys.modules["importlib"] + del sys.modules["importlib"] + yield + sys.modules["importlib"] = importlib + + @pytest.mark.parametrize( "attr,value,expected_value", ( @@ -49,7 +60,7 @@ def patched_pytest_module(monkeypatch): ("version_tuple", [3, 1, "0b2"], "3.1.0b2"), ), ) -def test_get_package_version(attr, value, expected_value): +def test_get_package_version(attr, value, expected_value, remove_importlib): # There is no file/module here, so we monkeypatch # pytest instead for our purposes setattr(pytest, attr, value) @@ -58,7 +69,7 @@ def test_get_package_version(attr, value, expected_value): delattr(pytest, attr) -def test_skips_version_callables(): +def test_skips_version_callables(remove_importlib): # There is no file/module here, so we monkeypatch # pytest instead for our purposes setattr(pytest, "version", lambda x: "1.2.3.4") @@ -81,7 +92,7 @@ def test_skips_version_callables(): ("version_tuple", [3, 1, "0b2"], (3, 1, "0b2")), ), ) -def test_get_package_version_tuple(attr, value, expected_value): +def test_get_package_version_tuple(attr, value, expected_value, remove_importlib): # There is no file/module here, so we monkeypatch # pytest instead for our purposes setattr(pytest, attr, value) From 54e8544e08fb7725c02d1bf4642f087e5311a308 Mon Sep 17 00:00:00 2001 From: Renan Ivo Date: Fri, 21 Jul 2023 23:56:19 -0300 Subject: [PATCH 2/6] Use get distribution name of module before fetching its version --- newrelic/common/package_version_utils.py | 4 +++- tests/agent_unittests/test_package_version_utils.py | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/newrelic/common/package_version_utils.py b/newrelic/common/package_version_utils.py index 4582387d6e..5ff1ac2e6d 100644 --- a/newrelic/common/package_version_utils.py +++ b/newrelic/common/package_version_utils.py @@ -74,7 +74,9 @@ def _get_package_version(name): # importlib was introduced into the standard library starting in Python3.8. if "importlib" in sys.modules and hasattr(sys.modules["importlib"], "metadata"): try: - version = sys.modules["importlib"].metadata.version(name) # pylint: disable=E1101 + distributions = sys.modules["importlib"].metadata.packages_distributions() + distribution_name = distributions.get(name, name) + version = sys.modules["importlib"].metadata.version(distribution_name) # pylint: disable=E1101 if version not in NULL_VERSIONS: return version except Exception: diff --git a/tests/agent_unittests/test_package_version_utils.py b/tests/agent_unittests/test_package_version_utils.py index db1d2d3fb7..3ce72f1447 100644 --- a/tests/agent_unittests/test_package_version_utils.py +++ b/tests/agent_unittests/test_package_version_utils.py @@ -103,6 +103,7 @@ def test_get_package_version_tuple(attr, value, expected_value, remove_importlib @SKIP_IF_NOT_IMPORTLIB_METADATA @validate_function_called("importlib.metadata", "version") +@validate_function_called("importlib.metadata", "packages_distributions") def test_importlib_metadata(): version = get_package_version("pytest") assert version not in NULL_VERSIONS, version From b8f65fc5842a5702189e4e9a04c77479333c455b Mon Sep 17 00:00:00 2001 From: Lalleh Rafeei Date: Tue, 15 Aug 2023 14:16:23 -0700 Subject: [PATCH 3/6] Add support for Python versions < 3.9 --- newrelic/common/package_version_utils.py | 15 ++++++++++----- .../test_package_version_utils.py | 17 ++++++++++++++++- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/newrelic/common/package_version_utils.py b/newrelic/common/package_version_utils.py index 5ff1ac2e6d..71f5772abe 100644 --- a/newrelic/common/package_version_utils.py +++ b/newrelic/common/package_version_utils.py @@ -74,11 +74,16 @@ def _get_package_version(name): # importlib was introduced into the standard library starting in Python3.8. if "importlib" in sys.modules and hasattr(sys.modules["importlib"], "metadata"): try: - distributions = sys.modules["importlib"].metadata.packages_distributions() - distribution_name = distributions.get(name, name) - version = sys.modules["importlib"].metadata.version(distribution_name) # pylint: disable=E1101 - if version not in NULL_VERSIONS: - return version + try: + # In Python3.10+ packages_distribution can be checked for as well + distributions = sys.modules["importlib"].metadata.packages_distributions() + distribution_name = distributions.get(name, name) + except: + distribution_name = name + finally: + version = sys.modules["importlib"].metadata.version(distribution_name) # pylint: disable=E1101 + if version not in NULL_VERSIONS: + return version except Exception: pass diff --git a/tests/agent_unittests/test_package_version_utils.py b/tests/agent_unittests/test_package_version_utils.py index 3ce72f1447..ce719649b3 100644 --- a/tests/agent_unittests/test_package_version_utils.py +++ b/tests/agent_unittests/test_package_version_utils.py @@ -24,11 +24,20 @@ get_package_version_tuple, ) +""" +importlib.metadata was a provisional addition to the std library in PY38 and PY39 +while pkg_resources was deprecated. +importlib.metadata is no longer provisional in PY310+. It added some attributes +such as distribution_packages and removed pkg_resources. +""" + IS_PY38_PLUS = sys.version_info[:2] >= (3, 8) +IS_PY310_PLUS = sys.version_info[:2] >= (3,10) SKIP_IF_NOT_IMPORTLIB_METADATA = pytest.mark.skipif(not IS_PY38_PLUS, reason="importlib.metadata is not supported.") SKIP_IF_IMPORTLIB_METADATA = pytest.mark.skipif( IS_PY38_PLUS, reason="importlib.metadata is preferred over pkg_resources." ) +SKIP_IF_NOT_PY310_PLUS = pytest.mark.skipif(not IS_PY310_PLUS, reason="These features were added in 3.10+") @pytest.fixture(scope="function", autouse=True) @@ -103,12 +112,18 @@ def test_get_package_version_tuple(attr, value, expected_value, remove_importlib @SKIP_IF_NOT_IMPORTLIB_METADATA @validate_function_called("importlib.metadata", "version") -@validate_function_called("importlib.metadata", "packages_distributions") def test_importlib_metadata(): version = get_package_version("pytest") assert version not in NULL_VERSIONS, version +@SKIP_IF_NOT_PY310_PLUS +@validate_function_called("importlib.metadata", "packages_distributions") +def test_mapping_import_to_distribution_packages(): + version = get_package_version("pytest") + assert version not in NULL_VERSIONS, version + + @SKIP_IF_IMPORTLIB_METADATA @validate_function_called("pkg_resources", "get_distribution") def test_pkg_resources_metadata(): From a3d4a40b7b2c70592a9351e7e246e3eb058ce912 Mon Sep 17 00:00:00 2001 From: Lalleh Rafeei Date: Tue, 15 Aug 2023 15:51:20 -0700 Subject: [PATCH 4/6] Fix conditional for packages_distributions --- newrelic/common/package_version_utils.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/newrelic/common/package_version_utils.py b/newrelic/common/package_version_utils.py index 71f5772abe..8c6aa42b19 100644 --- a/newrelic/common/package_version_utils.py +++ b/newrelic/common/package_version_utils.py @@ -74,16 +74,16 @@ def _get_package_version(name): # importlib was introduced into the standard library starting in Python3.8. if "importlib" in sys.modules and hasattr(sys.modules["importlib"], "metadata"): try: - try: - # In Python3.10+ packages_distribution can be checked for as well + # In Python3.10+ packages_distribution can be checked for as well + if hasattr(sys.modules["importlib"].metadata, "packages_distributions"): distributions = sys.modules["importlib"].metadata.packages_distributions() distribution_name = distributions.get(name, name) - except: + else: distribution_name = name - finally: - version = sys.modules["importlib"].metadata.version(distribution_name) # pylint: disable=E1101 - if version not in NULL_VERSIONS: - return version + + version = sys.modules["importlib"].metadata.version(distribution_name) # pylint: disable=E1101 + if version not in NULL_VERSIONS: + return version except Exception: pass From 8dd1c1b8cb91ef6bd244476a401de26f854bffd6 Mon Sep 17 00:00:00 2001 From: Lalleh Rafeei Date: Tue, 15 Aug 2023 16:02:53 -0700 Subject: [PATCH 5/6] Linter fixes --- newrelic/common/package_version_utils.py | 6 +++--- tests/agent_unittests/test_package_version_utils.py | 11 +++++------ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/newrelic/common/package_version_utils.py b/newrelic/common/package_version_utils.py index 8c6aa42b19..3152342b4d 100644 --- a/newrelic/common/package_version_utils.py +++ b/newrelic/common/package_version_utils.py @@ -75,12 +75,12 @@ def _get_package_version(name): if "importlib" in sys.modules and hasattr(sys.modules["importlib"], "metadata"): try: # In Python3.10+ packages_distribution can be checked for as well - if hasattr(sys.modules["importlib"].metadata, "packages_distributions"): - distributions = sys.modules["importlib"].metadata.packages_distributions() + if hasattr(sys.modules["importlib"].metadata, "packages_distributions"): # pylint: disable=E1101 + distributions = sys.modules["importlib"].metadata.packages_distributions() # pylint: disable=E1101 distribution_name = distributions.get(name, name) else: distribution_name = name - + version = sys.modules["importlib"].metadata.version(distribution_name) # pylint: disable=E1101 if version not in NULL_VERSIONS: return version diff --git a/tests/agent_unittests/test_package_version_utils.py b/tests/agent_unittests/test_package_version_utils.py index ce719649b3..0007623db7 100644 --- a/tests/agent_unittests/test_package_version_utils.py +++ b/tests/agent_unittests/test_package_version_utils.py @@ -24,12 +24,11 @@ get_package_version_tuple, ) -""" -importlib.metadata was a provisional addition to the std library in PY38 and PY39 -while pkg_resources was deprecated. -importlib.metadata is no longer provisional in PY310+. It added some attributes -such as distribution_packages and removed pkg_resources. -""" +# Notes: +# importlib.metadata was a provisional addition to the std library in PY38 and PY39 +# while pkg_resources was deprecated. +# importlib.metadata is no longer provisional in PY310+. It added some attributes +# such as distribution_packages and removed pkg_resources. IS_PY38_PLUS = sys.version_info[:2] >= (3, 8) IS_PY310_PLUS = sys.version_info[:2] >= (3,10) From dcb3dafe7143b9eabb3b526ecd18c2611773011c Mon Sep 17 00:00:00 2001 From: Lalleh Rafeei Date: Tue, 15 Aug 2023 16:39:00 -0700 Subject: [PATCH 6/6] Remove fixture in favor of test skips --- .../test_package_version_utils.py | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/tests/agent_unittests/test_package_version_utils.py b/tests/agent_unittests/test_package_version_utils.py index 0007623db7..30c22cff18 100644 --- a/tests/agent_unittests/test_package_version_utils.py +++ b/tests/agent_unittests/test_package_version_utils.py @@ -46,19 +46,10 @@ def patched_pytest_module(monkeypatch): monkeypatch.delattr(pytest, attr) yield pytest + - -@pytest.fixture(scope="function") -def remove_importlib(): - if "importlib" not in sys.modules: - return - - importlib = sys.modules["importlib"] - del sys.modules["importlib"] - yield - sys.modules["importlib"] = importlib - - +# This test only works on Python 3.7 +@SKIP_IF_IMPORTLIB_METADATA @pytest.mark.parametrize( "attr,value,expected_value", ( @@ -68,7 +59,7 @@ def remove_importlib(): ("version_tuple", [3, 1, "0b2"], "3.1.0b2"), ), ) -def test_get_package_version(attr, value, expected_value, remove_importlib): +def test_get_package_version(attr, value, expected_value): # There is no file/module here, so we monkeypatch # pytest instead for our purposes setattr(pytest, attr, value) @@ -77,7 +68,9 @@ def test_get_package_version(attr, value, expected_value, remove_importlib): delattr(pytest, attr) -def test_skips_version_callables(remove_importlib): +# This test only works on Python 3.7 +@SKIP_IF_IMPORTLIB_METADATA +def test_skips_version_callables(): # There is no file/module here, so we monkeypatch # pytest instead for our purposes setattr(pytest, "version", lambda x: "1.2.3.4") @@ -91,6 +84,8 @@ def test_skips_version_callables(remove_importlib): delattr(pytest, "version_tuple") +# This test only works on Python 3.7 +@SKIP_IF_IMPORTLIB_METADATA @pytest.mark.parametrize( "attr,value,expected_value", ( @@ -100,7 +95,7 @@ def test_skips_version_callables(remove_importlib): ("version_tuple", [3, 1, "0b2"], (3, 1, "0b2")), ), ) -def test_get_package_version_tuple(attr, value, expected_value, remove_importlib): +def test_get_package_version_tuple(attr, value, expected_value): # There is no file/module here, so we monkeypatch # pytest instead for our purposes setattr(pytest, attr, value)