From ae5a6ce6ab6d01a6bf23527aa07745675b40b282 Mon Sep 17 00:00:00 2001 From: Aleksander Wojnarowicz Date: Tue, 4 Jul 2023 19:19:26 +0200 Subject: [PATCH] misc fixes to automatic dependency tracking log and error instead of warning if path does not exist upload as file, not file set remove warning test case adjust unit tests test if error was logged with non existent file adjust units update changelog update changelog use neptune logger instead of logging.error use importlib instead of subprocess --- CHANGELOG.md | 10 +++++-- .../internal/utils/dependency_tracking.py | 24 ++++++++++----- tests/e2e/standard/test_init.py | 11 ++++--- .../utils/test_dependency_tracking.py | 30 +++++++++++-------- 4 files changed, 49 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 612746a88..7121f3df4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,11 @@ -## [UNRELEASED] neptune 1.3.2 +## [UNRELEASED] neptune 1.3.3 + +### Changes +- Dependency tracking feature will log an error if a given file path doesn't exist ([#1389](https://github.com/neptune-ai/neptune-client/pull/1389)) +- Use `pip list --format=freeze` instead of `pip freeze` in dependency tracking ([#1389](https://github.com/neptune-ai/neptune-client/pull/1389)) +- Log both uploaded and inferred requirements to the same namespace ([#1389](https://github.com/neptune-ai/neptune-client/pull/1389)) + +## neptune 1.3.2 ### Fixes - Fixed GitPython `is_dirty` failing on Windows ([#1371](https://github.com/neptune-ai/neptune-client/pull/1371)) @@ -11,7 +18,6 @@ ### Changes - Added support of writing to archived project exception ([#1355](https://github.com/neptune-ai/neptune-client/pull/1355)) -- Dependency tracking feature will raise warning if a given file path doesn't exist ([#1385](https://github.com/neptune-ai/neptune-client/pull/1385)) ## neptune 1.3.1 diff --git a/src/neptune/internal/utils/dependency_tracking.py b/src/neptune/internal/utils/dependency_tracking.py index 6752d952a..31a21dbd8 100644 --- a/src/neptune/internal/utils/dependency_tracking.py +++ b/src/neptune/internal/utils/dependency_tracking.py @@ -5,9 +5,7 @@ ] import os -import subprocess import sys -import warnings from abc import ( ABC, abstractmethod, @@ -17,6 +15,12 @@ Union, ) +if sys.version_info >= (3, 8): + from importlib.metadata import distributions +else: + from importlib_metadata import distributions + +from neptune.internal.utils.logger import logger from neptune.types import File if TYPE_CHECKING: @@ -31,10 +35,14 @@ def log_dependencies(self, run: "Run") -> None: class InferDependenciesStrategy(DependencyTrackingStrategy): def log_dependencies(self, run: "Run") -> None: - try: - dependencies_str = subprocess.check_output([sys.executable, "-m", "pip", "freeze"]).decode("utf-8") - except subprocess.SubprocessError: - return + dependencies = [] + dists = list(sorted(distributions(), key=lambda d: d.metadata["Name"])) + + for dist in dists: + name, version = dist.metadata["Name"], dist.metadata["Version"] + dependencies.append(f"{name}=={version}") + + dependencies_str = "\n".join(dependencies) if dependencies_str: run["source_code/requirements"].upload(File.from_content(dependencies_str)) @@ -46,6 +54,6 @@ def __init__(self, path: Union[str, os.PathLike]): def log_dependencies(self, run: "Run") -> None: if os.path.isfile(self._path): - run["source_code/files"].upload_files(os.path.basename(self._path)) + run["source_code/requirements"].upload(os.path.basename(self._path)) else: - warnings.warn(f"File '{self._path}' does not exist - skipping dependency file upload.") + logger.error("[ERROR] File '%s' does not exist - skipping dependency file upload.", self._path) diff --git a/tests/e2e/standard/test_init.py b/tests/e2e/standard/test_init.py index 060889ef5..2ff697545 100644 --- a/tests/e2e/standard/test_init.py +++ b/tests/e2e/standard/test_init.py @@ -105,10 +105,13 @@ def test_upload_dependency_file(self, environment): with zipped.open(filename, "r") as file: assert file.read().decode(encoding="utf-8") == "some-dependency==1.0.0" - def test_warning_raised_if_dependency_file_non_existent(self, environment): - with pytest.warns(Warning, match="^.*some_non_existent_file.*$"): - with neptune.init_run(dependencies="some_non_existent_file", project=environment.project): - ... + def test_warning_raised_if_dependency_file_non_existent(self, capsys, environment): + with neptune.init_run(dependencies="some_non_existent_file", project=environment.project): + ... + + captured = capsys.readouterr() + assert "'some_non_existent_file' does not exist" in captured.out + assert "ERROR" in captured.out def test_tracking_uncommitted_changes(self, repo, environment): file = repo.working_dir + "/some_file.txt" diff --git a/tests/unit/neptune/new/internal/utils/test_dependency_tracking.py b/tests/unit/neptune/new/internal/utils/test_dependency_tracking.py index e0eac6731..ae215e62a 100644 --- a/tests/unit/neptune/new/internal/utils/test_dependency_tracking.py +++ b/tests/unit/neptune/new/internal/utils/test_dependency_tracking.py @@ -1,3 +1,4 @@ +import itertools import unittest from unittest.mock import ( MagicMock, @@ -11,27 +12,32 @@ class TestDependencyTracking(unittest.TestCase): - @patch("subprocess.check_output", return_value=b"some_dependency==1.0.0\n") + @patch("neptune.internal.utils.dependency_tracking.distributions") @patch("neptune.types.File.from_content") - def test_infer_calls_upload_correctly(self, mock_from_content, mock_check_output): + def test_infer_calls_upload_correctly(self, mock_from_content, mock_distributions): + single_dist = MagicMock() + single_dist.metadata = {"Name": "some_dependency", "Version": "1.0.0"} + mock_distributions.return_value = itertools.chain([single_dist]) InferDependenciesStrategy().log_dependencies(run=MagicMock()) - mock_check_output.assert_called_once() - mock_from_content.assert_called_once_with("some_dependency==1.0.0\n") + mock_distributions.assert_called_once() + mock_from_content.assert_called_once_with("some_dependency==1.0.0") - @patch("subprocess.check_output", return_value=b"") + @patch("neptune.internal.utils.dependency_tracking.distributions", return_value=[]) @patch("neptune.types.File.from_content") - def test_infer_does_not_upload_empty_dependency_string(self, mock_from_content, mock_check_output): + def test_infer_does_not_upload_empty_dependency_string(self, mock_from_content, mock_distributions): InferDependenciesStrategy().log_dependencies(run=MagicMock()) - mock_check_output.assert_called_once() + mock_distributions.assert_called_once() mock_from_content.assert_not_called() - @patch("neptune.handler.Handler.upload_files") - def test_file_strategy_not_uploading_if_path_incorrect(self, mock_upload_files): + @patch("neptune.handler.Handler.upload") + @patch("neptune.internal.utils.dependency_tracking.logger") + def test_file_strategy_path_incorrect(self, mock_logger, mock_upload): FileDependenciesStrategy(path="non-existent_file_path.txt").log_dependencies(run=MagicMock()) - mock_upload_files.assert_not_called() + mock_upload.assert_not_called() + mock_logger.error.assert_called_once() @patch("os.path.isfile", return_value=True) def test_file_strategy_uploads_correct_path(self, mock_is_file): @@ -39,8 +45,8 @@ def test_file_strategy_uploads_correct_path(self, mock_is_file): handler = MagicMock() run.__getitem__ = MagicMock() run.__getitem__.return_value = handler - handler.upload_files = MagicMock() + handler.upload = MagicMock() FileDependenciesStrategy(path="valid_file_path.txt").log_dependencies(run=run) - handler.upload_files.assert_called_once_with("valid_file_path.txt") + handler.upload.assert_called_once_with("valid_file_path.txt")