From 6e0c9c24b433e120a23db99a22a1da81d18ce93f Mon Sep 17 00:00:00 2001 From: Panos Vagenas <35837085+vagenas@users.noreply.github.com> Date: Thu, 19 Oct 2023 15:31:41 +0200 Subject: [PATCH 1/2] fix: handle invalid profiles, improve CLI output Signed-off-by: Panos Vagenas <35837085+vagenas@users.noreply.github.com> --- .gitignore | 2 +- deepsearch/core/cli/profile.py | 7 ++++- deepsearch/core/cli/utils.py | 5 ++-- deepsearch/core/client/settings.py | 7 ++++- deepsearch/core/client/settings_manager.py | 33 +++++++++++++--------- deepsearch/cps/client/api.py | 3 +- 6 files changed, 38 insertions(+), 19 deletions(-) diff --git a/.gitignore b/.gitignore index 05eb6b2c..06e4e93b 100644 --- a/.gitignore +++ b/.gitignore @@ -247,7 +247,7 @@ dmypy.json # Virtualenv # http://iamzed.com/2009/05/07/a-primer-on-virtualenv/ pyvenv.cfg -.env +*.env .venv env/ venv/ diff --git a/deepsearch/core/cli/profile.py b/deepsearch/core/cli/profile.py index 71b81ded..9da3e0c6 100644 --- a/deepsearch/core/cli/profile.py +++ b/deepsearch/core/cli/profile.py @@ -11,7 +11,7 @@ ) from deepsearch.core.cli.utils import cli_handler from deepsearch.core.client.settings import ProfileSettings -from deepsearch.core.client.settings_manager import settings_mgr +from deepsearch.core.client.settings_manager import SettingsManager app = typer.Typer(no_args_is_help=True) @@ -35,6 +35,7 @@ def add_profile( ), activate_profile: bool = typer.Option(default=True), ): + settings_mgr = SettingsManager() prfl_name = ( profile_name if profile_name else settings_mgr.get_profile_name_suggestion() ) @@ -63,6 +64,7 @@ def list_profiles() -> None: "active", "profile", ) + settings_mgr = SettingsManager() profiles = settings_mgr.get_all_profile_settings() active_profile = settings_mgr.get_active_profile() @@ -98,6 +100,7 @@ def show_profile( "profile", "config", ) + settings_mgr = SettingsManager() prfl_name = profile_name or settings_mgr.get_active_profile() profile = settings_mgr.get_profile_settings(profile_name=prfl_name) @@ -117,6 +120,7 @@ def show_profile( def set_default_profile( profile_name: str, ) -> None: + settings_mgr = SettingsManager() settings_mgr.activate_profile(profile_name=profile_name) @@ -129,4 +133,5 @@ def set_default_profile( def remove_profile( profile_name: str, ) -> None: + settings_mgr = SettingsManager() settings_mgr.remove_profile(profile_name=profile_name) diff --git a/deepsearch/core/cli/utils.py b/deepsearch/core/cli/utils.py index 12be3447..a1cd215d 100644 --- a/deepsearch/core/cli/utils.py +++ b/deepsearch/core/cli/utils.py @@ -2,7 +2,7 @@ import typer -from deepsearch.core.client.settings_manager import settings_mgr +from deepsearch.core.client.settings import CLISettings def cli_handler(): @@ -14,7 +14,8 @@ def wrap(*args, **kwargs): try: return func(*args, **kwargs) except Exception as e: - if settings_mgr.get_show_cli_stack_traces(): + settings = CLISettings() + if settings.show_cli_stack_traces: raise e else: typer.secho(str(e), fg=typer.colors.RED) diff --git a/deepsearch/core/client/settings.py b/deepsearch/core/client/settings.py index ee09283f..550c5ddc 100644 --- a/deepsearch/core/client/settings.py +++ b/deepsearch/core/client/settings.py @@ -53,8 +53,13 @@ def from_cli_prompt(cls) -> ProfileSettings: class MainSettings(DumpableSettings): + profile: Optional[str] = None - profile: Optional[str] = None # None only when profiles not yet iniitialized + class Config: + env_prefix = "DEEPSEARCH_" + + +class CLISettings(DumpableSettings): show_cli_stack_traces: bool = False class Config: diff --git a/deepsearch/core/client/settings_manager.py b/deepsearch/core/client/settings_manager.py index 318fa17b..4db7b843 100644 --- a/deepsearch/core/client/settings_manager.py +++ b/deepsearch/core/client/settings_manager.py @@ -2,7 +2,7 @@ from dataclasses import dataclass from enum import Enum from pathlib import Path -from typing import Dict, Optional +from typing import Dict, List, Optional import platformdirs from pydantic import ValidationError @@ -23,13 +23,13 @@ class KnownProfile(str, Enum): SDS = "sds" - # DS_EXPERIENCE = "ds-experience" # TODO: uncomment once applicable + DS_EXPERIENCE = "ds-experience" DS_INTERNAL = "ds-internal" HOST_BY_PROFILE = { KnownProfile.SDS.value: "https://sds.app.accelerate.science", - # KnownProfile.DS_EXPERIENCE.value: "https://deepsearch-experience.res.ibm.com", # TODO: uncomment once applicable + KnownProfile.DS_EXPERIENCE.value: "https://deepsearch-experience.res.ibm.com", KnownProfile.DS_INTERNAL.value: "https://cps.foc-deepsearch.zurich.ibm.com", } @@ -61,15 +61,21 @@ def __init__(self) -> None: # initialize internal profile cache from Pydantic Settings based on dotenv self._profile_cache: Dict[str, ProfileSettingsEntry] = {} + paths_to_remove: List[Path] = [] for f in os.listdir(self._profile_root_path): file_path = self._profile_root_path / f if file_path.suffix == ".env": profile_name = file_path.stem - self._profile_cache[profile_name] = ProfileSettingsEntry( - path=file_path, - settings=ProfileSettings(_env_file=file_path), # type: ignore + try: + profile_settings = ProfileSettings(_env_file=file_path) # type: ignore # suppressing due to known Pydantic bug https://github.com/pydantic/pydantic/issues/3072 - ) + + self._profile_cache[profile_name] = ProfileSettingsEntry( + path=file_path, + settings=profile_settings, + ) + except ValidationError: + paths_to_remove.append(file_path) # reset any stale active profile config if ( @@ -93,6 +99,13 @@ def __init__(self) -> None: self._migrate_legacy_config() + if paths_to_remove: + for path_to_rem in paths_to_remove: + path_to_rem.unlink() + profile_names = ", ".join([f'"{p.stem}"' for p in paths_to_remove]) + err_msg = f"Following profiles were invalid and have been removed: {profile_names}; if needed please re-add" + raise RuntimeError(err_msg) + def _migrate_legacy_config(self) -> None: if self._main_settings.profile is None: legacy_cfg_path = self.config_root_path / LEGACY_CFG_FILENAME @@ -197,9 +210,3 @@ def remove_profile(self, profile_name: str) -> None: prfl_settgs_entry = self._profile_cache.pop(profile_name) # update cache prfl_settgs_entry.path.unlink() # remove file - - def get_show_cli_stack_traces(self) -> bool: - return self._main_settings.show_cli_stack_traces - - -settings_mgr = SettingsManager() diff --git a/deepsearch/cps/client/api.py b/deepsearch/cps/client/api.py index 69862c82..670c55fb 100644 --- a/deepsearch/cps/client/api.py +++ b/deepsearch/cps/client/api.py @@ -12,7 +12,7 @@ DeepSearchKeyAuth, ) from deepsearch.core.client.settings import ProfileSettings -from deepsearch.core.client.settings_manager import settings_mgr +from deepsearch.core.client.settings_manager import SettingsManager from deepsearch.cps.apis import public as sw_client from deepsearch.cps.client.components import ( CpsApiDataCatalogs, @@ -164,6 +164,7 @@ def from_env(cls, profile_name: Optional[str] = None) -> CpsApi: try: settings = ProfileSettings() except ValidationError: + settings_mgr = SettingsManager() settings = settings_mgr.get_profile_settings(profile_name=profile_name) return cls.from_settings(settings=settings) From 1424acafc7a35770ebb308736944156894b2ce9b Mon Sep 17 00:00:00 2001 From: Panos Vagenas <35837085+vagenas@users.noreply.github.com> Date: Fri, 20 Oct 2023 08:45:27 +0200 Subject: [PATCH 2/2] expedite handling Signed-off-by: Panos Vagenas <35837085+vagenas@users.noreply.github.com> --- deepsearch/core/client/settings_manager.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/deepsearch/core/client/settings_manager.py b/deepsearch/core/client/settings_manager.py index 4db7b843..ac836c09 100644 --- a/deepsearch/core/client/settings_manager.py +++ b/deepsearch/core/client/settings_manager.py @@ -77,6 +77,14 @@ def __init__(self) -> None: except ValidationError: paths_to_remove.append(file_path) + # handle any invalid profiles + if paths_to_remove: + for path_to_rem in paths_to_remove: + path_to_rem.unlink() + profile_names = ", ".join([f'"{p.stem}"' for p in paths_to_remove]) + err_msg = f"Following profiles were invalid and have been removed: {profile_names}; if needed please re-add" + raise RuntimeError(err_msg) + # reset any stale active profile config if ( self._main_settings.profile is not None @@ -99,13 +107,6 @@ def __init__(self) -> None: self._migrate_legacy_config() - if paths_to_remove: - for path_to_rem in paths_to_remove: - path_to_rem.unlink() - profile_names = ", ".join([f'"{p.stem}"' for p in paths_to_remove]) - err_msg = f"Following profiles were invalid and have been removed: {profile_names}; if needed please re-add" - raise RuntimeError(err_msg) - def _migrate_legacy_config(self) -> None: if self._main_settings.profile is None: legacy_cfg_path = self.config_root_path / LEGACY_CFG_FILENAME