fixed uninstall, list validators cli cmds, and tests

Author: AlejandroEsquivel

guardrails/cli/hub/list.py

@@ -2,7 +2,6 @@
 import re
 
 from guardrails.cli.hub.hub import hub_command
-from guardrails.cli.hub.utils import get_site_packages_location
 from guardrails.hub_telemetry.hub_tracing import trace
 from .console import console
 
@@ -11,7 +10,9 @@
 @trace(name="guardrails-cli/hub/list")
 def list():
     """List all installed validators."""
-    site_packages = get_site_packages_location()
+    from guardrails.hub.validator_package_service import ValidatorPackageService
+
+    site_packages = ValidatorPackageService.get_site_packages_location()
     hub_init_file = os.path.join(site_packages, "guardrails", "hub", "__init__.py")
 
     installed_validators = []

guardrails/cli/hub/uninstall.py

@@ -1,5 +1,4 @@
 import os
-import shutil
 import sys
 from typing import List, Literal
 
@@ -10,9 +9,7 @@
 from guardrails.cli.server.hub_client import get_validator_manifest
 from guardrails_hub_types import Manifest
 
-from guardrails.cli.hub.utils import get_site_packages_location
-from guardrails.cli.hub.utils import get_org_and_package_dirs
-from guardrails.cli.hub.utils import get_hub_directory
+from guardrails.cli.hub.utils import pip_process
 from guardrails.hub_telemetry.hub_tracing import trace
 
 from .console import console
@@ -32,46 +29,28 @@ def remove_line(file_path: str, line_content: str):
 
 
 def remove_from_hub_inits(manifest: Manifest, site_packages: str):
-    org_package = get_org_and_package_dirs(manifest)
+    from guardrails.hub.validator_package_service import ValidatorPackageService
+
     exports: List[str] = manifest.exports or []
     sorted_exports = sorted(exports, reverse=True)
-    module_name = manifest.module_name
-    relative_path = ".".join([*org_package, module_name])
-    import_line = (
-        f"from guardrails.hub.{relative_path} import {', '.join(sorted_exports)}"
+
+    validator_id = manifest.id
+    import_path = ValidatorPackageService.get_import_path_from_validator_id(
+        validator_id
     )
+    import_line = f"from {import_path} import {', '.join(sorted_exports)}"
 
     # Remove import line from main __init__.py
     hub_init_location = os.path.join(site_packages, "guardrails", "hub", "__init__.py")
     remove_line(hub_init_location, import_line)
 
-    # Remove import line from namespace __init__.py
-    namespace = org_package[0]
-    namespace_init_location = os.path.join(
-        site_packages, "guardrails", "hub", namespace, "__init__.py"
-    )
-    lines = remove_line(namespace_init_location, import_line)
-
-    # remove namespace pkg if namespace __init__.py is empty
-    if (len(lines) == 0) and (namespace != "hub"):
-        logger.info(f"Removing namespace package {namespace} as it is now empty")
-        try:
-            shutil.rmtree(os.path.join(site_packages, "guardrails", "hub", namespace))
-        except Exception as e:
-            logger.error(f"Error removing namespace package {namespace}")
-            logger.error(e)
-            sys.exit(1)
-
-
-def uninstall_hub_module(manifest: Manifest, site_packages: str):
-    uninstall_directory = get_hub_directory(manifest, site_packages)
-    logger.info(f"Removing directory {uninstall_directory}")
-    try:
-        shutil.rmtree(uninstall_directory)
-    except Exception as e:
-        logger.error("Error removing directory")
-        logger.error(e)
-        sys.exit(1)
+
+def uninstall_hub_module(manifest: Manifest):
+    from guardrails.hub.validator_package_service import ValidatorPackageService
+
+    validator_id = manifest.id
+    package_name = ValidatorPackageService.get_normalized_package_name(validator_id)
+    pip_process("uninstall", package_name, flags=["-y"], quiet=True)
 
 
 @hub_command.command()
@@ -82,6 +61,9 @@ def uninstall(
     ),
 ):
     """Uninstall a validator from the Hub."""
+    from guardrails.hub.validator_package_service import ValidatorPackageService
+
+    print(f"ValidatorPackageService: {ValidatorPackageService}")
     if not package_uri.startswith("hub://"):
         logger.error("Invalid URI!")
         sys.exit(1)
@@ -98,11 +80,11 @@ def uninstall(
     # Prep
     with console.status("Fetching manifest", spinner="bouncingBar"):
         module_manifest = get_validator_manifest(module_name)
-        site_packages = get_site_packages_location()
+        site_packages = ValidatorPackageService.get_site_packages_location()
 
     # Uninstall
     with console.status("Removing module", spinner="bouncingBar"):
-        uninstall_hub_module(module_manifest, site_packages)
+        uninstall_hub_module(module_manifest)
 
     # Cleanup
     with console.status("Cleaning up", spinner="bouncingBar"):

guardrails/cli/hub/utils.py

@@ -58,7 +58,7 @@ def pip_process(
             except Exception:
                 logger.debug(
                     f"JSON parse exception in decoding output from pip {action}"
-                    f"{package}. Falling back to accumulating the byte stream",
+                    f" {package}. Falling back to accumulating the byte stream",
                 )
             accumulator = {}
             parsed = BytesHeaderParser().parsebytes(result.stdout.encode())
@@ -77,20 +77,13 @@ def pip_process(
                 f"stdout: {(exc.stdout or '').strip()}"
             )
         )
-        # Re-raise the error or handle it accordingly
-        raise
+        sys.exit(1)
     except Exception as e:
         logger.error(
             f"An unexpected exception occurred while trying to {action} {package}!",
             e,
         )
-        raise
-
-
-def get_site_packages_location() -> str:
-    output = pip_process("show", "pip", format=json_format)
-    pip_location = output["Location"]  # type: ignore
-    return pip_location
+        sys.exit(1)
 
 
 def get_org_and_package_dirs(manifest: Manifest) -> List[str]:

tests/unit_tests/cli/hub/test_install.py

@@ -1,4 +1,4 @@
-from unittest.mock import ANY, call
+from unittest.mock import ANY, MagicMock, call
 from typer.testing import CliRunner
 from guardrails.cli.hub.install import hub_command
 
@@ -140,25 +140,27 @@ def test_no_package_string_format(self, mocker):
 
         mock_sys_executable = mocker.patch("guardrails.cli.hub.utils.sys.executable")
 
-        mock_subprocess_check_output = mocker.patch(
-            "guardrails.cli.hub.utils.subprocess.check_output"
-        )
-        mock_subprocess_check_output.return_value = str.encode("string output")
+        mock_subprocess_run = mocker.patch("guardrails.cli.hub.utils.subprocess.run")
+        subprocess_result_mock = MagicMock()
+        subprocess_result_mock.stdout = "string output"
+        mock_subprocess_run.return_value = subprocess_result_mock
 
         from guardrails.cli.hub.utils import pip_process
 
         response = pip_process("inspect", flags=["--path=./install-here"])
 
-        assert mock_logger_debug.call_count == 2
+        assert mock_logger_debug.call_count == 1
         debug_calls = [
             call("running pip inspect --path=./install-here "),
-            call("decoding output from pip inspect "),
         ]
         mock_logger_debug.assert_has_calls(debug_calls)
 
-        mock_subprocess_check_output.assert_called_once_with(
+        mock_subprocess_run.assert_called_once_with(
             [mock_sys_executable, "-m", "pip", "inspect", "--path=./install-here"],
             env={},
+            capture_output=True,
+            text=True,
+            check=True,
         )
 
         assert response == "string output"
@@ -169,10 +171,11 @@ def test_json_format(self, mocker):
 
         mock_sys_executable = mocker.patch("guardrails.cli.hub.utils.sys.executable")
 
-        mock_subprocess_check_output = mocker.patch(
-            "guardrails.cli.hub.utils.subprocess.check_output"
-        )
-        mock_subprocess_check_output.return_value = str.encode("json output")
+        mock_subprocess_run = mocker.patch("guardrails.cli.hub.utils.subprocess.run")
+        subprocess_result_mock = MagicMock()
+        subprocess_result_mock.stdout = "json outout"
+
+        mock_subprocess_run.return_value = subprocess_result_mock
 
         class MockBytesHeaderParser:
             def parsebytes(self, *args):
@@ -186,18 +189,21 @@ def parsebytes(self, *args):
 
         response = pip_process("show", "pip", format="json")
 
-        assert mock_logger_debug.call_count == 3
+        assert mock_logger_debug.call_count == 2
         debug_calls = [
             call("running pip show  pip"),
-            call("decoding output from pip show pip"),
             call(
                 "JSON parse exception in decoding output from pip show pip. Falling back to accumulating the byte stream"  # noqa
             ),
         ]
         mock_logger_debug.assert_has_calls(debug_calls)
 
-        mock_subprocess_check_output.assert_called_once_with(
-            [mock_sys_executable, "-m", "pip", "show", "pip"], env={}
+        mock_subprocess_run.assert_called_once_with(
+            [mock_sys_executable, "-m", "pip", "show", "pip"],
+            env={},
+            capture_output=True,
+            text=True,
+            check=True,
         )
 
         assert response == {"output": "json"}
@@ -271,16 +277,3 @@ def test_install_with_upgrade_flag(self, mocker):
         )
 
         assert result.exit_code == 0
-
-
-def test_get_site_packages_location(mocker):
-    mock_pip_process = mocker.patch("guardrails.cli.hub.utils.pip_process")
-    mock_pip_process.return_value = {"Location": "/site-packages"}
-
-    from guardrails.cli.hub.utils import get_site_packages_location
-
-    response = get_site_packages_location()
-
-    mock_pip_process.assert_called_once_with("show", "pip", format="json")
-
-    assert response == "/site-packages"

tests/unit_tests/cli/hub/test_list.py

@@ -13,7 +13,7 @@ class TestListCommand:
     @pytest.fixture(autouse=True)
     def setup(self, mocker):
         mocker.patch(
-            "guardrails.cli.hub.utils.get_site_packages_location",
+            "guardrails.hub.validator_package_service.ValidatorPackageService.get_manifest_and_site_packages",
             return_value="/test/site-packages",
         )
 

tests/unit_tests/cli/hub/test_uninstall.py

@@ -5,46 +5,35 @@
 from guardrails_hub_types import Manifest
 from guardrails.cli.hub.uninstall import remove_from_hub_inits
 
-manifest_mock = Manifest(
-    encoder="some_encoder",
-    id="module_id",
-    name="test_module",
-    author={"name": "Author Name", "email": "[email protected]"},
-    maintainers=[{"name": "Maintainer Name", "email": "[email protected]"}],
-    repository={"url": "https://github.com/example/repo"},
-    namespace="guardrails",
-    package_name="test_package",
-    description="Test module",
-    module_name="test_module",
-    exports=["Validator", "Helper"],
+manifest_mock = Manifest.from_dict(
+    {
+        "id": "guardrails/test_package",
+        "name": "test_module",
+        "author": {"name": "Author Name", "email": "[email protected]"},
+        "maintainers": [{"name": "Maintainer Name", "email": "[email protected]"}],
+        "repository": {"url": "https://github.com/example/repo"},
+        "packageName": "test_package",
+        "moduleName": "test_module",
+        "namespace": "guardrails",
+        "description": "Test module",
+        "exports": ["Validator", "Helper"],
+    }
 )
 
 
 def test_remove_from_hub_inits(mocker):
-    mocker.patch(
-        "guardrails.cli.hub.uninstall.get_org_and_package_dirs",
-        return_value=["guardrails", "test_package"],
-    )
     mock_remove_line = mocker.patch("guardrails.cli.hub.uninstall.remove_line")
-    mock_remove_dirs = mocker.patch("shutil.rmtree")
 
     remove_from_hub_inits(manifest_mock, "/site-packages")
 
     expected_calls = [
         call(
             "/site-packages/guardrails/hub/__init__.py",
-            "from guardrails.hub.guardrails.test_package.test_module import "
-            "Validator, Helper",
-        ),
-        call(
-            "/site-packages/guardrails/hub/guardrails/__init__.py",
-            "from guardrails.hub.guardrails.test_package.test_module import "
-            "Validator, Helper",
+            "from guardrails_grhub_test_package import " "Validator, Helper",
         ),
     ]
 
     mock_remove_line.assert_has_calls(expected_calls, any_order=True)
-    mock_remove_dirs.assert_called_once_with("/site-packages/guardrails/hub/guardrails")
 
 
 def test_uninstall_invalid_uri(mocker):
@@ -81,10 +70,7 @@ def test_uninstall_valid_uri(mocker):
         "guardrails.cli.hub.uninstall.get_validator_manifest",
         return_value=manifest_mock,
     )
-    mocker.patch(
-        "guardrails.cli.hub.uninstall.get_site_packages_location",
-        return_value="/site-packages",
-    )
+
     mock_uninstall_hub_module = mocker.patch(
         "guardrails.cli.hub.uninstall.uninstall_hub_module"
     )
@@ -93,9 +79,27 @@ def test_uninstall_valid_uri(mocker):
     )
     mocker.patch("guardrails.cli.hub.uninstall.console")
 
+    validator_package_service_mock = mocker.patch(
+        "guardrails.hub.validator_package_service.ValidatorPackageService",
+    )
+    validator_package_service_mock.get_site_packages_location.return_value = (
+        "/site-packages"
+    )
+
     from guardrails.cli.hub.uninstall import uninstall
 
     uninstall("hub://guardrails/test-validator")
 
-    mock_uninstall_hub_module.assert_called_once_with(manifest_mock, "/site-packages")
+    mock_uninstall_hub_module.assert_called_once_with(manifest_mock)
     mock_remove_from_hub_inits.assert_called_once_with(manifest_mock, "/site-packages")
+
+
+def test_uninstall_hub_module(mocker):
+    mock_pip_process = mocker.patch("guardrails.cli.hub.uninstall.pip_process")
+    from guardrails.cli.hub.uninstall import uninstall_hub_module
+
+    uninstall_hub_module(manifest_mock)
+
+    mock_pip_process.assert_called_once_with(
+        "uninstall", "guardrails-grhub-test-package", flags=["-y"], quiet=True
+    )