From f3b54d5bd4ec1c7eeedea4df7e3df7d79c8210d2 Mon Sep 17 00:00:00 2001 From: Andrii Ieroshenko Date: Thu, 9 Nov 2023 16:02:54 -0800 Subject: [PATCH 1/6] if ids are not getting associated with models, set them to None --- .../jupyter-ai/jupyter_ai/config_manager.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/jupyter-ai/jupyter_ai/config_manager.py b/packages/jupyter-ai/jupyter_ai/config_manager.py index 5adc604cf..63bb003f8 100644 --- a/packages/jupyter-ai/jupyter_ai/config_manager.py +++ b/packages/jupyter-ai/jupyter_ai/config_manager.py @@ -169,6 +169,25 @@ def _init_config(self): ) config.embeddings_provider_id = None + # if the currently selected language or embedding model ids are + # not associated with models, set them to `None` and log a warning. + if ( + lm_id is not None + and not get_lm_provider(lm_id, self._lm_providers)[1] + ): + self.log.warning( + f"No language model is associated with '{lm_id}'. Setting to None." + ) + config.model_provider_id = None + if ( + em_id is not None + and not get_em_provider(em_id, self._em_providers)[1] + ): + self.log.warning( + f"No embedding model is associated with '{em_id}'. Setting to None." + ) + config.embeddings_provider_id = None + # re-write to the file to validate the config and apply any # updates to the config file immediately self._write_config(config) From ceacc53f3678ac082cecdc1e0fec5b7d73f516ae Mon Sep 17 00:00:00 2001 From: Andrii Ieroshenko Date: Fri, 10 Nov 2023 17:07:48 -0800 Subject: [PATCH 2/6] add test_handle_bad_provider_ids --- .../jupyter_ai/tests/test_config_manager.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py b/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py index dafba7adf..376142c05 100644 --- a/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py +++ b/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py @@ -3,6 +3,7 @@ import os import pytest +from unittest.mock import mock_open, patch from jupyter_ai.config_manager import ( AuthError, ConfigManager, @@ -83,6 +84,11 @@ def reset(config_path, schema_path): pass +@pytest.fixture +def config_with_bad_provider_ids(): + return json.dumps({"model_provider_id:": "foo", "embeddings_provider_id": "bar"}) + + def configure_to_cohere(cm: ConfigManager): """Configures the ConfigManager to use Cohere language and embedding models with the API key set. Returns a 3-tuple of the keyword arguments used.""" @@ -290,3 +296,11 @@ def test_forbid_deleting_key_in_use(cm: ConfigManager): with pytest.raises(KeyInUseError): cm.delete_api_key("COHERE_API_KEY") + + +def test_handle_bad_provider_ids(config_with_bad_provider_ids, common_cm_kwargs): + with patch("builtins.open", mock_open(read_data=config_with_bad_provider_ids)): + cm = ConfigManager(**common_cm_kwargs) + config_desc = cm.get_config() + assert config_desc.model_provider_id is None + assert config_desc.embeddings_provider_id is None From 1f8784b8d83cfff59b252282ef1191b9df624614 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 11 Nov 2023 02:09:14 +0000 Subject: [PATCH 3/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py b/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py index 376142c05..63f6fffc8 100644 --- a/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py +++ b/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py @@ -1,9 +1,9 @@ import json import logging import os +from unittest.mock import mock_open, patch import pytest -from unittest.mock import mock_open, patch from jupyter_ai.config_manager import ( AuthError, ConfigManager, From df7212004b329b1bf5a25886f348aa8e1e409cc4 Mon Sep 17 00:00:00 2001 From: Andrii Ieroshenko Date: Fri, 10 Nov 2023 18:24:29 -0800 Subject: [PATCH 4/6] fix tests --- .../jupyter_ai/tests/test_config_manager.py | 33 ++++++++++++++----- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py b/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py index 63f6fffc8..0d109ca09 100644 --- a/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py +++ b/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py @@ -10,7 +10,7 @@ KeyInUseError, WriteConflictError, ) -from jupyter_ai.models import DescribeConfigResponse, UpdateConfigRequest +from jupyter_ai.models import DescribeConfigResponse, GlobalConfig, UpdateConfigRequest from jupyter_ai_magics.utils import get_em_providers, get_lm_providers from pydantic import ValidationError @@ -85,8 +85,25 @@ def reset(config_path, schema_path): @pytest.fixture -def config_with_bad_provider_ids(): - return json.dumps({"model_provider_id:": "foo", "embeddings_provider_id": "bar"}) +def config_with_bad_provider_ids(tmp_path): + """Fixture that creates a `config.json` file with bad provider ids in `tmp_path` folder and returns path to the file.""" + config_data = { + "model_provider_id:": "foo:bar", + "embeddings_provider_id": "buzz:fizz", + "api_keys": {}, + "send_with_shift_enter": False, + "fields": {}, + } + config_path = tmp_path / "config.json" + with open(config_path, "w") as file: + json.dump(config_data, file) + return str(config_path) + + +@pytest.fixture +def cm_with_bad_provider_ids(common_cm_kwargs, config_with_bad_provider_ids): + common_cm_kwargs["config_path"] = config_with_bad_provider_ids + return ConfigManager(**common_cm_kwargs) def configure_to_cohere(cm: ConfigManager): @@ -298,9 +315,7 @@ def test_forbid_deleting_key_in_use(cm: ConfigManager): cm.delete_api_key("COHERE_API_KEY") -def test_handle_bad_provider_ids(config_with_bad_provider_ids, common_cm_kwargs): - with patch("builtins.open", mock_open(read_data=config_with_bad_provider_ids)): - cm = ConfigManager(**common_cm_kwargs) - config_desc = cm.get_config() - assert config_desc.model_provider_id is None - assert config_desc.embeddings_provider_id is None +def test_handle_bad_provider_ids(cm_with_bad_provider_ids): + config_desc = cm_with_bad_provider_ids.get_config() + assert config_desc.model_provider_id is None + assert config_desc.embeddings_provider_id is None From 0117ba5496c8117fc9c582f240ea9a4ffd982ca8 Mon Sep 17 00:00:00 2001 From: Andrii Ieroshenko Date: Fri, 10 Nov 2023 18:31:47 -0800 Subject: [PATCH 5/6] Add docstrings --- packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py b/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py index 0d109ca09..7cbde64f2 100644 --- a/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py +++ b/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py @@ -86,7 +86,7 @@ def reset(config_path, schema_path): @pytest.fixture def config_with_bad_provider_ids(tmp_path): - """Fixture that creates a `config.json` file with bad provider ids in `tmp_path` folder and returns path to the file.""" + """Fixture that creates a `config.json` with model_provider_id and embeddings_provider_id values that would not associate with models. File is created in `tmp_path` folder. Function returns path to the file.""" config_data = { "model_provider_id:": "foo:bar", "embeddings_provider_id": "buzz:fizz", @@ -102,6 +102,7 @@ def config_with_bad_provider_ids(tmp_path): @pytest.fixture def cm_with_bad_provider_ids(common_cm_kwargs, config_with_bad_provider_ids): + """Config manager instance created with `config_path` set to mocked `config.json` with model_provider_id and embeddings_provider_id values that would not associate with models.""" common_cm_kwargs["config_path"] = config_with_bad_provider_ids return ConfigManager(**common_cm_kwargs) From 7516d56a5882ba5f6fc98567271ad399c75893d5 Mon Sep 17 00:00:00 2001 From: Andrii Ieroshenko Date: Fri, 10 Nov 2023 18:46:28 -0800 Subject: [PATCH 6/6] adjust docstrings --- packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py b/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py index 7cbde64f2..8fa59ab6a 100644 --- a/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py +++ b/packages/jupyter-ai/jupyter_ai/tests/test_config_manager.py @@ -86,7 +86,7 @@ def reset(config_path, schema_path): @pytest.fixture def config_with_bad_provider_ids(tmp_path): - """Fixture that creates a `config.json` with model_provider_id and embeddings_provider_id values that would not associate with models. File is created in `tmp_path` folder. Function returns path to the file.""" + """Fixture that creates a `config.json` with `model_provider_id` and `embeddings_provider_id` values that would not associate with models. File is created in `tmp_path` folder. Function returns path to the file.""" config_data = { "model_provider_id:": "foo:bar", "embeddings_provider_id": "buzz:fizz", @@ -102,7 +102,7 @@ def config_with_bad_provider_ids(tmp_path): @pytest.fixture def cm_with_bad_provider_ids(common_cm_kwargs, config_with_bad_provider_ids): - """Config manager instance created with `config_path` set to mocked `config.json` with model_provider_id and embeddings_provider_id values that would not associate with models.""" + """Config manager instance created with `config_path` set to mocked `config.json` with `model_provider_id` and `embeddings_provider_id` values that would not associate with models.""" common_cm_kwargs["config_path"] = config_with_bad_provider_ids return ConfigManager(**common_cm_kwargs)