diff --git a/guardrails/hub/validator_package_service.py b/guardrails/hub/validator_package_service.py index 8431dc9d1..41518bc3b 100644 --- a/guardrails/hub/validator_package_service.py +++ b/guardrails/hub/validator_package_service.py @@ -208,21 +208,6 @@ def get_validator_id(validator_uri: str): return (validator_id, validator_version) - @staticmethod - def get_install_url(manifest: Manifest) -> str: - repo = manifest.repository - repo_url = repo.url - branch = repo.branch - - git_url = repo_url - if not repo_url.startswith("git+"): - git_url = f"git+{repo_url}" - - if branch is not None: - git_url = f"{git_url}@{branch}" - - return git_url - @staticmethod def run_post_install( manifest: Manifest, site_packages: str, logger=guardrails_logger @@ -271,11 +256,6 @@ def run_post_install( """ ) - @staticmethod - def get_hub_directory(manifest: Manifest, site_packages: str) -> str: - org_package = ValidatorPackageService.get_org_and_package_dirs(manifest) - return os.path.join(site_packages, "guardrails", "hub", *org_package) - @staticmethod def get_normalized_package_name(validator_id: str): validator_id_parts = validator_id.split("/") diff --git a/tests/unit_tests/hub/test_validator_package_service.py b/tests/unit_tests/hub/test_validator_package_service.py index a407c312b..7f27d9c79 100644 --- a/tests/unit_tests/hub/test_validator_package_service.py +++ b/tests/unit_tests/hub/test_validator_package_service.py @@ -1,4 +1,5 @@ from pathlib import Path +from typing import cast import pytest import sys from unittest.mock import call, patch, MagicMock @@ -88,6 +89,7 @@ def test_closes_early_if_already_added(self, mocker): from guardrails.hub.validator_package_service import ValidatorPackageService + manifest = cast(Manifest, manifest) ValidatorPackageService.add_to_hub_inits(manifest, site_packages) assert mock_open.call_count == 2 @@ -154,6 +156,7 @@ def test_appends_import_line_if_not_present(self, mocker): from guardrails.hub.validator_package_service import ValidatorPackageService + manifest = cast(Manifest, manifest) ValidatorPackageService.add_to_hub_inits(manifest, site_packages) assert mock_open.call_count == 2 @@ -235,6 +238,7 @@ def test_creates_namespace_init_if_not_exists(self, mocker): from guardrails.hub.validator_package_service import ValidatorPackageService + manifest = cast(Manifest, manifest) ValidatorPackageService.add_to_hub_inits(manifest, site_packages) assert mock_open.call_count == 2 @@ -335,7 +339,7 @@ class TestRunPostInstall: [ Manifest.from_dict( { - "id": "id", + "id": "guardrails-ai/id", "name": "name", "author": {"name": "me", "email": "me@me.me"}, "maintainers": [], @@ -350,7 +354,7 @@ class TestRunPostInstall: ), Manifest.from_dict( { - "id": "id", + "id": "guardrails-ai/id", "name": "name", "author": {"name": "me", "email": "me@me.me"}, "maintainers": [], @@ -391,7 +395,7 @@ def test_runs_script_if_exists(self, mocker): manifest = Manifest.from_dict( { - "id": "id", + "id": "guardrails-ai/id", "name": "name", "author": {"name": "me", "email": "me@me.me"}, "maintainers": [], @@ -406,6 +410,7 @@ def test_runs_script_if_exists(self, mocker): } ) + manifest = cast(Manifest, manifest) ValidatorPackageService.run_post_install(manifest, "./site_packages") assert mock_subprocess_check_output.call_count == 1 @@ -421,7 +426,7 @@ class TestValidatorPackageService: def setup_method(self): self.manifest = Manifest.from_dict( { - "id": "id", + "id": "guardrails/id", "name": "name", "author": {"name": "me", "email": "me@me.me"}, "maintainers": [], @@ -466,25 +471,17 @@ def test_get_site_packages_location(self, mock_get_module_path): site_packages_path = ValidatorPackageService.get_site_packages_location() assert site_packages_path == "/fake/site-packages" - @patch( - "guardrails.hub.validator_package_service.ValidatorPackageService.get_org_and_package_dirs" - ) @patch( "guardrails.hub.validator_package_service.ValidatorPackageService.reload_module" ) - def test_get_validator_from_manifest( - self, mock_reload_module, mock_get_org_and_package_dirs - ): - mock_get_org_and_package_dirs.return_value = ["guardrails_ai", "test_package"] - + def test_get_validator_from_manifest(self, mock_reload_module): mock_validator_module = MagicMock() mock_reload_module.return_value = mock_validator_module - ValidatorPackageService.get_validator_from_manifest(self.manifest) + manifest = cast(Manifest, self.manifest) + ValidatorPackageService.get_validator_from_manifest(manifest) - mock_reload_module.assert_called_once_with( - f"guardrails.hub.guardrails_ai.test_package.{self.manifest.module_name}" - ) + mock_reload_module.assert_called_once_with("guardrails_grhub_id") @pytest.mark.parametrize( "manifest,expected", @@ -492,7 +489,7 @@ def test_get_validator_from_manifest( ( Manifest.from_dict( { - "id": "id", + "id": "guardrails-ai/id", "name": "name", "author": {"name": "me", "email": "me@me.me"}, "maintainers": [], @@ -510,12 +507,12 @@ def test_get_validator_from_manifest( ( Manifest.from_dict( { - "id": "id", + "id": "guardrails-ai/id", "name": "name", "author": {"name": "me", "email": "me@me.me"}, "maintainers": [], "repository": {"url": "some-repo"}, - "namespace": "", + "namespace": "guardrails-ai", "packageName": "test-validator", "moduleName": "test_validator", "description": "description", @@ -534,98 +531,17 @@ def test_get_org_and_package_dirs(self, manifest, expected): assert actual == expected def test_get_module_name_valid(self): - module_name = ValidatorPackageService.get_module_name("hub://test-module") + module_name, module_version = ValidatorPackageService.get_validator_id( + "hub://test-module>=1.0.0" + ) assert module_name == "test-module" + assert module_version == ">=1.0.0" def test_get_module_name_invalid(self): with pytest.raises(InvalidHubInstallURL): - ValidatorPackageService.get_module_name("invalid-uri") - - @pytest.mark.parametrize( - "manifest,expected", - [ - ( - Manifest.from_dict( - { - "id": "id", - "name": "name", - "author": {"name": "me", "email": "me@me.me"}, - "maintainers": [], - "repository": {"url": "some-repo"}, - "namespace": "guardrails-ai", - "packageName": "test-validator", - "moduleName": "validator", - "description": "description", - "exports": ["TestValidator"], - "tags": {}, - } - ), - "git+some-repo", - ), - ( - Manifest.from_dict( - { - "id": "id", - "name": "name", - "author": {"name": "me", "email": "me@me.me"}, - "maintainers": [], - "repository": {"url": "git+some-repo"}, - "namespace": "guardrails-ai", - "packageName": "test-validator", - "moduleName": "validator", - "description": "description", - "exports": ["TestValidator"], - "tags": {}, - "post_install": "", - } - ), - "git+some-repo", - ), - ( - Manifest.from_dict( - { - "id": "id", - "name": "name", - "author": {"name": "me", "email": "me@me.me"}, - "maintainers": [], - "repository": {"url": "git+some-repo", "branch": "prod"}, - "namespace": "guardrails-ai", - "packageName": "test-validator", - "moduleName": "validator", - "description": "description", - "exports": ["TestValidator"], - "tags": {}, - "post_install": "", - } - ), - "git+some-repo@prod", - ), - ], - ) - def test_get_install_url(self, manifest, expected): - actual = ValidatorPackageService.get_install_url(manifest) - assert actual == expected - - def test_get_hub_directory(self): - hub_directory = ValidatorPackageService.get_hub_directory( - self.manifest, self.site_packages - ) - assert ( - hub_directory - == "./.venv/lib/python3.X/site-packages/guardrails/hub/guardrails/test_validator" # noqa - ) # noqa + ValidatorPackageService.get_validator_id("invalid-uri") def test_install_hub_module(self, mocker): - mock_get_install_url = mocker.patch( - "guardrails.hub.validator_package_service.ValidatorPackageService.get_install_url" - ) - mock_get_install_url.return_value = "mock-install-url" - - mock_get_hub_directory = mocker.patch( - "guardrails.hub.validator_package_service.ValidatorPackageService.get_hub_directory" - ) - mock_get_hub_directory.return_value = "mock/install/directory" - mock_pip_process = mocker.patch( "guardrails.hub.validator_package_service.pip_process" ) @@ -653,7 +569,7 @@ def test_install_hub_module(self, mocker): manifest = Manifest.from_dict( { - "id": "id", + "id": "guardrails-ai/id", "name": "name", "author": {"name": "me", "email": "me@me.me"}, "maintainers": [], @@ -666,13 +582,10 @@ def test_install_hub_module(self, mocker): "tags": {}, } ) - site_packages = "./site-packages" - ValidatorPackageService.install_hub_module(manifest, site_packages) + manifest = cast(Manifest, manifest) + ValidatorPackageService.install_hub_module(manifest.id) - mock_get_install_url.assert_called_once_with(manifest) - mock_get_hub_directory.assert_called_once_with(manifest, site_packages) - - assert mock_pip_process.call_count == 5 + assert mock_pip_process.call_count == 1 pip_calls = [ call( "install", @@ -680,15 +593,5 @@ def test_install_hub_module(self, mocker): ["--target=mock/install/directory", "--no-deps"], quiet=False, ), - call( - "inspect", - flags=["--path=mock/install/directory"], - format="json", - quiet=False, - no_color=True, - ), - call("install", "rstr", quiet=False), - call("install", "openai<2", quiet=False), - call("install", "pydash>=7.0.6,<8.0.0", quiet=False), ] mock_pip_process.assert_has_calls(pip_calls)