From 8c99b371e0d34afdf5c101d9c9419ea70a712545 Mon Sep 17 00:00:00 2001 From: Bhaskar Gurram Date: Sat, 26 Sep 2026 17:22:30 +0000 Subject: [PATCH 1/2] Declare requests as an install dependency for telemetry `import olive` imports `olive.telemetry`, whose OneCollector exporter (`olive/telemetry/library/exporter.py`, `options.py`, `transport.py`) imports `requests` at module level. `requests` was never listed in `requirements.txt`, so it only ended up installed as a transitive dependency of `transformers`/`huggingface_hub`. Newer releases of those packages no longer depend on `requests`, so a fresh `pip install olive-ai` (no extras) fails with `ModuleNotFoundError: No module named 'requests'` on `import olive`. Telemetry is a core, always-on component (its other dependency, `opentelemetry-sdk`, is already a hard requirement and the exporter's public API is typed around `requests.Session`), so declare `requests` in `requirements.txt` rather than making the import optional. Add a test that statically checks every unconditional third-party import under `olive/telemetry` maps to a distribution declared in `requirements.txt`, so a future telemetry dependency cannot silently break `import olive` again. --- requirements.txt | 1 + test/telemetry/__init__.py | 4 ++ test/telemetry/test_dependencies.py | 81 +++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+) create mode 100644 test/telemetry/__init__.py create mode 100644 test/telemetry/test_dependencies.py diff --git a/requirements.txt b/requirements.txt index 661e166586..90851e413b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,6 +10,7 @@ pandas pydantic>=2.0 pyyaml questionary +requests torch torchmetrics>=1.0.0 transformers diff --git a/test/telemetry/__init__.py b/test/telemetry/__init__.py new file mode 100644 index 0000000000..862c45ce31 --- /dev/null +++ b/test/telemetry/__init__.py @@ -0,0 +1,4 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# -------------------------------------------------------------------------- diff --git a/test/telemetry/test_dependencies.py b/test/telemetry/test_dependencies.py new file mode 100644 index 0000000000..0c61341c47 --- /dev/null +++ b/test/telemetry/test_dependencies.py @@ -0,0 +1,81 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# -------------------------------------------------------------------------- +"""Guard against olive.telemetry importing packages that are not declared install dependencies. + +``olive.telemetry`` is imported by ``import olive`` itself, so every third-party module it +imports unconditionally must be listed in ``requirements.txt``. Otherwise a plain +``pip install olive-ai`` (no extras) is broken at import time. +""" + +import ast +import re +import sys +from importlib.metadata import packages_distributions +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +TELEMETRY_ROOT = REPO_ROOT / "olive" / "telemetry" +REQUIREMENTS_TXT = REPO_ROOT / "requirements.txt" + + +def _normalize(name: str) -> str: + """Normalize a distribution name as described in PEP 503.""" + return re.sub(r"[-_.]+", "-", name).lower() + + +def _declared_requirements() -> set[str]: + declared = set() + for raw_line in REQUIREMENTS_TXT.read_text(encoding="utf-8").splitlines(): + line = raw_line.split("#", 1)[0].strip() + match = re.match(r"[A-Za-z0-9][A-Za-z0-9._-]*", line) + if match: + declared.add(_normalize(match.group(0))) + return declared + + +def _unconditional_top_level_imports(path: Path) -> set[str]: + """Return the top-level package names a module imports at module scope without any guard. + + Only statements directly in the module body count: imports inside ``try``/``except``, + ``if TYPE_CHECKING:`` or functions are not hard dependencies. + """ + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + names = set() + for node in tree.body: + if isinstance(node, ast.Import): + names.update(alias.name.split(".")[0] for alias in node.names) + elif isinstance(node, ast.ImportFrom) and node.level == 0 and node.module: + names.add(node.module.split(".")[0]) + return names + + +def _third_party_imports() -> dict[str, set[str]]: + """Map each third-party top-level module to the telemetry files that import it.""" + imports: dict[str, set[str]] = {} + for path in sorted(TELEMETRY_ROOT.rglob("*.py")): + for name in _unconditional_top_level_imports(path): + if name == "olive" or name in sys.stdlib_module_names: + continue + imports.setdefault(name, set()).add(path.relative_to(REPO_ROOT).as_posix()) + return imports + + +@pytest.mark.skipif(not REQUIREMENTS_TXT.exists(), reason="requires a source checkout of the repository") +def test_telemetry_imports_are_declared_in_requirements(): + declared = _declared_requirements() + distributions = packages_distributions() + + undeclared = {} + for module, files in _third_party_imports().items(): + candidates = {_normalize(dist) for dist in distributions.get(module, [module])} + if not candidates & declared: + undeclared[module] = sorted(files) + + assert not undeclared, ( + "olive.telemetry is imported by `import olive`, but these modules are not declared in " + f"requirements.txt: {undeclared}" + ) From c029af3301746cc4ba339c6d63634924c70d02df Mon Sep 17 00:00:00 2001 From: Bhaskar Gurram Date: Wed, 7 Oct 2026 21:39:41 +0000 Subject: [PATCH 2/2] Remove telemetry dependency test --- test/telemetry/__init__.py | 4 -- test/telemetry/test_dependencies.py | 81 ----------------------------- 2 files changed, 85 deletions(-) delete mode 100644 test/telemetry/__init__.py delete mode 100644 test/telemetry/test_dependencies.py diff --git a/test/telemetry/__init__.py b/test/telemetry/__init__.py deleted file mode 100644 index 862c45ce31..0000000000 --- a/test/telemetry/__init__.py +++ /dev/null @@ -1,4 +0,0 @@ -# ------------------------------------------------------------------------- -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. -# -------------------------------------------------------------------------- diff --git a/test/telemetry/test_dependencies.py b/test/telemetry/test_dependencies.py deleted file mode 100644 index 0c61341c47..0000000000 --- a/test/telemetry/test_dependencies.py +++ /dev/null @@ -1,81 +0,0 @@ -# ------------------------------------------------------------------------- -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. -# -------------------------------------------------------------------------- -"""Guard against olive.telemetry importing packages that are not declared install dependencies. - -``olive.telemetry`` is imported by ``import olive`` itself, so every third-party module it -imports unconditionally must be listed in ``requirements.txt``. Otherwise a plain -``pip install olive-ai`` (no extras) is broken at import time. -""" - -import ast -import re -import sys -from importlib.metadata import packages_distributions -from pathlib import Path - -import pytest - -REPO_ROOT = Path(__file__).resolve().parents[2] -TELEMETRY_ROOT = REPO_ROOT / "olive" / "telemetry" -REQUIREMENTS_TXT = REPO_ROOT / "requirements.txt" - - -def _normalize(name: str) -> str: - """Normalize a distribution name as described in PEP 503.""" - return re.sub(r"[-_.]+", "-", name).lower() - - -def _declared_requirements() -> set[str]: - declared = set() - for raw_line in REQUIREMENTS_TXT.read_text(encoding="utf-8").splitlines(): - line = raw_line.split("#", 1)[0].strip() - match = re.match(r"[A-Za-z0-9][A-Za-z0-9._-]*", line) - if match: - declared.add(_normalize(match.group(0))) - return declared - - -def _unconditional_top_level_imports(path: Path) -> set[str]: - """Return the top-level package names a module imports at module scope without any guard. - - Only statements directly in the module body count: imports inside ``try``/``except``, - ``if TYPE_CHECKING:`` or functions are not hard dependencies. - """ - tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) - names = set() - for node in tree.body: - if isinstance(node, ast.Import): - names.update(alias.name.split(".")[0] for alias in node.names) - elif isinstance(node, ast.ImportFrom) and node.level == 0 and node.module: - names.add(node.module.split(".")[0]) - return names - - -def _third_party_imports() -> dict[str, set[str]]: - """Map each third-party top-level module to the telemetry files that import it.""" - imports: dict[str, set[str]] = {} - for path in sorted(TELEMETRY_ROOT.rglob("*.py")): - for name in _unconditional_top_level_imports(path): - if name == "olive" or name in sys.stdlib_module_names: - continue - imports.setdefault(name, set()).add(path.relative_to(REPO_ROOT).as_posix()) - return imports - - -@pytest.mark.skipif(not REQUIREMENTS_TXT.exists(), reason="requires a source checkout of the repository") -def test_telemetry_imports_are_declared_in_requirements(): - declared = _declared_requirements() - distributions = packages_distributions() - - undeclared = {} - for module, files in _third_party_imports().items(): - candidates = {_normalize(dist) for dist in distributions.get(module, [module])} - if not candidates & declared: - undeclared[module] = sorted(files) - - assert not undeclared, ( - "olive.telemetry is imported by `import olive`, but these modules are not declared in " - f"requirements.txt: {undeclared}" - )