From 4793924f648159f66098930acb1d72e2643c00f2 Mon Sep 17 00:00:00 2001 From: Caleb Courier Date: Mon, 29 Jul 2024 10:58:24 -0500 Subject: [PATCH 1/2] fix use_remote_endpoint, better install tests, increase type version range --- guardrails/cli/hub/install.py | 2 +- poetry.lock | 5 +- pyproject.toml | 2 +- tests/unit_tests/cli/hub/test_install.py | 145 +++++++++++++++++++++-- 4 files changed, 142 insertions(+), 12 deletions(-) diff --git a/guardrails/cli/hub/install.py b/guardrails/cli/hub/install.py index 08205c53b..c4fb3dbf6 100644 --- a/guardrails/cli/hub/install.py +++ b/guardrails/cli/hub/install.py @@ -257,7 +257,7 @@ def do_nothing_context(*args, **kwargs): if has_rc_file: # if we do want to remote then we don't want to install local models use_remote_endpoint = ( - not Credentials.from_rc_file(logger).use_remote_inferencing + Credentials.from_rc_file(logger).use_remote_inferencing and module_has_endpoint ) elif install_local_models is None and module_has_endpoint: diff --git a/poetry.lock b/poetry.lock index 56ef1229e..b07f4b585 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.7.1 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand. [[package]] name = "aiohttp" @@ -3861,6 +3861,7 @@ description = "Nvidia JIT LTO Library" optional = true python-versions = ">=3" files = [ + {file = "nvidia_nvjitlink_cu12-12.5.40-py3-none-manylinux2014_aarch64.whl", hash = "sha256:004186d5ea6a57758fd6d57052a123c73a4815adf365eb8dd6a85c9eaa7535ff"}, {file = "nvidia_nvjitlink_cu12-12.5.40-py3-none-manylinux2014_x86_64.whl", hash = "sha256:d9714f27c1d0f0895cd8915c07a87a1d0029a0aa36acaf9156952ec2a8a12189"}, {file = "nvidia_nvjitlink_cu12-12.5.40-py3-none-win_amd64.whl", hash = "sha256:c3401dc8543b52d3a8158007a0c1ab4e9c768fcbd24153a48c86972102197ddd"}, ] @@ -7268,4 +7269,4 @@ vectordb = ["faiss-cpu", "numpy"] [metadata] lock-version = "2.0" python-versions = "^3.9" -content-hash = "7324b8f6722a8ffa85dce351e32df205cd72d6f7dd24d37186c4a71604dcd489" +content-hash = "769202aeb0eeee6e828412b291aacc5ca20fd05e78eef2ed1d6aea3fdb7fdf28" diff --git a/pyproject.toml b/pyproject.toml index 2742a0d41..dee859b5f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,7 @@ lxml = "^4.9.3" openai = "^1.30.1" rich = "^13.6.0" pydantic = ">=2.0.0, <3.0" -typer = {extras = ["all"], version = "^0.9.0"} +typer = {extras = ["all"], version = ">=0.9.0, <0.13"} griffe = "^0.36.9" tenacity = ">=8.1.0" regex = "^2023.10.3" diff --git a/tests/unit_tests/cli/hub/test_install.py b/tests/unit_tests/cli/hub/test_install.py index 77c2c795d..6e4f41ad9 100644 --- a/tests/unit_tests/cli/hub/test_install.py +++ b/tests/unit_tests/cli/hub/test_install.py @@ -3,15 +3,25 @@ import pytest from typer.testing import CliRunner +from guardrails.classes.credentials import Credentials from guardrails.cli.hub.install import hub_command, install from guardrails.cli.server.module_manifest import ModuleManifest from tests.unit_tests.mocks.mock_file import MockFile +@pytest.mark.parametrize( + "use_remote_inferencing", + [False, True], +) class TestInstall: - def test_exits_early_if_uri_is_not_valid(self, mocker): + def test_exits_early_if_uri_is_not_valid(self, mocker, use_remote_inferencing): mock_logger_error = mocker.patch("guardrails.cli.hub.install.logger.error") + mocker.patch( + "guardrails.cli.hub.install.Credentials.has_rc_file", + return_value=True, + ) + from guardrails.cli.hub.install import sys sys_exit_spy = mocker.spy(sys, "exit") @@ -22,9 +32,21 @@ def test_exits_early_if_uri_is_not_valid(self, mocker): mock_logger_error.assert_called_once_with("Invalid URI!") sys_exit_spy.assert_called_once_with(1) - def test_install_local_models__false(self, mocker, monkeypatch): + def test_install_local_models__false( + self, mocker, monkeypatch, use_remote_inferencing + ): mock_logger_log = mocker.patch("guardrails.cli.hub.install.logger.log") + mocker.patch( + "guardrails.cli.hub.install.Credentials.has_rc_file", + return_value=True, + ) + + mocker.patch( + "guardrails.cli.hub.install.Credentials.from_rc_file", + return_value=Credentials(use_remote_inferencing=use_remote_inferencing), + ) + mock_get_validator_manifest = mocker.patch( "guardrails.cli.hub.install.get_validator_manifest" ) @@ -86,9 +108,21 @@ def test_install_local_models__false(self, mocker, monkeypatch): mock_add_to_hub_init.assert_called_once_with(manifest, site_packages) - def test_install_local_models__true(self, mocker, monkeypatch): + def test_install_local_models__true( + self, mocker, monkeypatch, use_remote_inferencing + ): mock_logger_log = mocker.patch("guardrails.cli.hub.install.logger.log") + mocker.patch( + "guardrails.cli.hub.install.Credentials.has_rc_file", + return_value=True, + ) + + mocker.patch( + "guardrails.cli.hub.install.Credentials.from_rc_file", + return_value=Credentials(use_remote_inferencing=use_remote_inferencing), + ) + mock_get_validator_manifest = mocker.patch( "guardrails.cli.hub.install.get_validator_manifest" ) @@ -149,9 +183,21 @@ def test_install_local_models__true(self, mocker, monkeypatch): mock_add_to_hub_init.assert_called_once_with(manifest, site_packages) - def test_install_local_models__none(self, mocker, monkeypatch): + def test_install_local_models__none( + self, mocker, monkeypatch, use_remote_inferencing + ): mock_logger_log = mocker.patch("guardrails.cli.hub.install.logger.log") + mocker.patch( + "guardrails.cli.hub.install.Credentials.has_rc_file", + return_value=True, + ) + + mocker.patch( + "guardrails.cli.hub.install.Credentials.from_rc_file", + return_value=Credentials(use_remote_inferencing=use_remote_inferencing), + ) + mock_get_validator_manifest = mocker.patch( "guardrails.cli.hub.install.get_validator_manifest" ) @@ -209,9 +255,19 @@ def test_install_local_models__none(self, mocker, monkeypatch): mock_add_to_hub_init.assert_called_once_with(manifest, site_packages) - def test_happy_path(self, mocker, monkeypatch): + def test_happy_path(self, mocker, monkeypatch, use_remote_inferencing): mock_logger_log = mocker.patch("guardrails.cli.hub.install.logger.log") + mocker.patch( + "guardrails.cli.hub.install.Credentials.has_rc_file", + return_value=True, + ) + + mocker.patch( + "guardrails.cli.hub.install.Credentials.from_rc_file", + return_value=Credentials(use_remote_inferencing=use_remote_inferencing), + ) + mock_get_validator_manifest = mocker.patch( "guardrails.cli.hub.install.get_validator_manifest" ) @@ -226,7 +282,7 @@ def test_happy_path(self, mocker, monkeypatch): "package_name": "test-validator", "module_name": "test_validator", "exports": ["TestValidator"], - "tags": {"has_guardrails_endpoint": True}, + "tags": {"has_guardrails_endpoint": False}, } ) mock_get_validator_manifest.return_value = manifest @@ -260,12 +316,16 @@ def test_happy_path(self, mocker, monkeypatch): assert mock_get_site_packages_location.call_count == 1 - def test_install_local_models_confirmation(self, mocker): + def test_install_local_models_confirmation(self, mocker, use_remote_inferencing): # Mock dependencies mocker.patch("guardrails.cli.hub.install.get_site_packages_location") mocker.patch("guardrails.cli.hub.install.install_hub_module") mocker.patch("guardrails.cli.hub.install.run_post_install") mocker.patch("guardrails.cli.hub.install.add_to_hub_inits") + mocker.patch( + "guardrails.cli.hub.install.Credentials.has_rc_file", + return_value=False, + ) # Create a manifest with Guardrails endpoint manifest_with_endpoint = ModuleManifest.from_dict( @@ -279,7 +339,7 @@ def test_install_local_models_confirmation(self, mocker): "package_name": "test-package", "module_name": "test_module", "exports": ["TestValidator"], - "tags": {"has_guardrails_endpoint": False}, + "tags": {"has_guardrails_endpoint": True}, } ) mocker.patch( @@ -297,6 +357,75 @@ def test_install_local_models_confirmation(self, mocker): # Check if the installation was successful assert result.exit_code == 0 + def test_use_remote_endpoint( + self, mocker, monkeypatch, use_remote_inferencing: bool + ): + mock_logger_log = mocker.patch("guardrails.cli.hub.install.logger.log") + + mocker.patch( + "guardrails.cli.hub.install.Credentials.has_rc_file", + return_value=True, + ) + + mocker.patch( + "guardrails.cli.hub.install.Credentials.from_rc_file", + return_value=Credentials(use_remote_inferencing=use_remote_inferencing), + ) + + mock_get_validator_manifest = mocker.patch( + "guardrails.cli.hub.install.get_validator_manifest" + ) + manifest = ModuleManifest.from_dict( + { + "id": "id", + "name": "name", + "author": {"name": "me", "email": "me@me.me"}, + "maintainers": [], + "repository": {"url": "some-repo"}, + "namespace": "guardrails", + "package_name": "test-validator", + "module_name": "test_validator", + "exports": ["TestValidator"], + "tags": {"has_guardrails_endpoint": True}, + } + ) + mock_get_validator_manifest.return_value = manifest + + mock_get_site_packages_location = mocker.patch( + "guardrails.cli.hub.install.get_site_packages_location" + ) + site_packages = "./.venv/lib/python3.X/site-packages" + mock_get_site_packages_location.return_value = site_packages + + mocker.patch("guardrails.cli.hub.install.install_hub_module") + mocker.patch("guardrails.cli.hub.install.run_post_install") + mocker.patch("guardrails.cli.hub.install.add_to_hub_inits") + + runner = CliRunner() + + runner.invoke(hub_command, ["install", "hub://guardrails/test-validator"]) + + msg = ( + "Skipping post install, models will not be downloaded for local inference." + if use_remote_inferencing + else "Installing models locally!" + ) + + log_calls = [ + call(level=5, msg="Installing hub://guardrails/test-validator..."), + call( + level=5, + msg=msg, # noqa + ), # noqa + ] + + assert mock_logger_log.call_count == 3 + mock_logger_log.assert_has_calls(log_calls) + + mock_get_validator_manifest.assert_called_once_with("guardrails/test-validator") + + assert mock_get_site_packages_location.call_count == 1 + class TestPipProcess: def test_no_package_string_format(self, mocker): From f715ca4543876bed7b9f6a30bf7e2602e4619304 Mon Sep 17 00:00:00 2001 From: Caleb Courier Date: Mon, 29 Jul 2024 11:10:04 -0500 Subject: [PATCH 2/2] work around encoder requirement for py 3.9 --- tests/unit_tests/cli/hub/test_install.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tests/unit_tests/cli/hub/test_install.py b/tests/unit_tests/cli/hub/test_install.py index 6e4f41ad9..8daa692bc 100644 --- a/tests/unit_tests/cli/hub/test_install.py +++ b/tests/unit_tests/cli/hub/test_install.py @@ -44,7 +44,9 @@ def test_install_local_models__false( mocker.patch( "guardrails.cli.hub.install.Credentials.from_rc_file", - return_value=Credentials(use_remote_inferencing=use_remote_inferencing), + return_value=Credentials.from_dict( + {"use_remote_inferencing": use_remote_inferencing} + ), ) mock_get_validator_manifest = mocker.patch( @@ -120,7 +122,9 @@ def test_install_local_models__true( mocker.patch( "guardrails.cli.hub.install.Credentials.from_rc_file", - return_value=Credentials(use_remote_inferencing=use_remote_inferencing), + return_value=Credentials.from_dict( + {"use_remote_inferencing": use_remote_inferencing} + ), ) mock_get_validator_manifest = mocker.patch( @@ -195,7 +199,9 @@ def test_install_local_models__none( mocker.patch( "guardrails.cli.hub.install.Credentials.from_rc_file", - return_value=Credentials(use_remote_inferencing=use_remote_inferencing), + return_value=Credentials.from_dict( + {"use_remote_inferencing": use_remote_inferencing} + ), ) mock_get_validator_manifest = mocker.patch( @@ -265,7 +271,9 @@ def test_happy_path(self, mocker, monkeypatch, use_remote_inferencing): mocker.patch( "guardrails.cli.hub.install.Credentials.from_rc_file", - return_value=Credentials(use_remote_inferencing=use_remote_inferencing), + return_value=Credentials.from_dict( + {"use_remote_inferencing": use_remote_inferencing} + ), ) mock_get_validator_manifest = mocker.patch( @@ -369,7 +377,9 @@ def test_use_remote_endpoint( mocker.patch( "guardrails.cli.hub.install.Credentials.from_rc_file", - return_value=Credentials(use_remote_inferencing=use_remote_inferencing), + return_value=Credentials.from_dict( + {"use_remote_inferencing": use_remote_inferencing} + ), ) mock_get_validator_manifest = mocker.patch(