Skip to content

Commit

Permalink
misc fixes to automatic dependency tracking
Browse files Browse the repository at this point in the history
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
  • Loading branch information
AleksanderWWW committed Jul 6, 2023
1 parent b1bedc9 commit ae5a6ce
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 26 deletions.
10 changes: 8 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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))
Expand All @@ -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

Expand Down
24 changes: 16 additions & 8 deletions src/neptune/internal/utils/dependency_tracking.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
]

import os
import subprocess
import sys
import warnings
from abc import (
ABC,
abstractmethod,
Expand All @@ -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:
Expand All @@ -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))
Expand All @@ -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)
11 changes: 7 additions & 4 deletions tests/e2e/standard/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
30 changes: 18 additions & 12 deletions tests/unit/neptune/new/internal/utils/test_dependency_tracking.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import itertools
import unittest
from unittest.mock import (
MagicMock,
Expand All @@ -11,36 +12,41 @@


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):
run = MagicMock()
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")

0 comments on commit ae5a6ce

Please sign in to comment.