From 92536e9ba507a57048bdb1f1e7c66e9cfadeea75 Mon Sep 17 00:00:00 2001 From: Kieran Manning Date: Sat, 12 Apr 2025 18:36:13 +0100 Subject: [PATCH 1/2] chore: first pass adding warnings. Includes .envrc uv fix --- .envrc | 19 ++++++-- cloudsmith_cli/cli/commands/repos.py | 4 ++ cloudsmith_cli/cli/config.py | 65 +++++++++++++++++++++++++++- cloudsmith_cli/cli/decorators.py | 25 +++++++++++ 4 files changed, 108 insertions(+), 5 deletions(-) diff --git a/.envrc b/.envrc index 9d01ead4..a5ae4ac0 100644 --- a/.envrc +++ b/.envrc @@ -18,9 +18,22 @@ if [ -d ".venv" ]; then fi fi -python -m uv venv .venv --python $PYTHON_VERSION -python -m uv pip install -U pip uv -python -m uv pip install -e . +if [ ! -d ".venv" ] +then + # System python doesn't like us installing packages into it + # Test if uv is already installed (via brew or other package manager etc.) + # and use that if available. Otherwise fall back to previous behvaiour + if command -v uv + then + uv venv .venv --python $PYTHON_VERSION + uv pip install -U pip uv + uv pip install -e . + else + python -m uv venv .venv --python $PYTHON_VERSION + python -m uv pip install -U pip uv + python -m uv pip install -e . + fi +fi source ./.venv/bin/activate diff --git a/cloudsmith_cli/cli/commands/repos.py b/cloudsmith_cli/cli/commands/repos.py index db11c148..79c12c9d 100644 --- a/cloudsmith_cli/cli/commands/repos.py +++ b/cloudsmith_cli/cli/commands/repos.py @@ -60,6 +60,7 @@ def print_repositories(opts, data, page_info=None, show_list_info=True): @decorators.common_cli_output_options @decorators.common_api_auth_options @decorators.initialise_api +@decorators.verify_authenticated @click.pass_context def repositories(ctx, opts): # pylink: disable=unused-argument """ @@ -101,6 +102,9 @@ def get(ctx, opts, owner_repo, page, page_size): click.echo("Getting list of repositories ... ", nl=False, err=use_stderr) + repo = None + owner = None + if isinstance(owner_repo, list): if len(owner_repo) == 1: owner = owner_repo[0] diff --git a/cloudsmith_cli/cli/config.py b/cloudsmith_cli/cli/config.py index fdb9ae19..20c054d6 100644 --- a/cloudsmith_cli/cli/config.py +++ b/cloudsmith_cli/cli/config.py @@ -77,6 +77,7 @@ class ConfigReader(ConfigFileReader): config_name = "standard" config_searchpath = list(_CFG_SEARCH_PATHS) config_section_schemas = [ConfigSchema.Default, ConfigSchema.Profile] + config_warning_issued = False @classmethod def select_config_schema_for(cls, section_name): @@ -147,6 +148,19 @@ def has_default_file(cls): return False + @classmethod + def config_already_warned(cls): + """ + Check if a configuration file warning has been issued. + This is required as configs are gathered at the root of the + command chain as well as for command verbs + """ + if cls.config_warning_issued: + return True + + cls.config_warning_issued = True + return False + @classmethod def load_config(cls, opts, path=None, profile=None): """Load a configuration file into an options object.""" @@ -161,8 +175,32 @@ def load_config(cls, opts, path=None, profile=None): cls._load_values_into_opts(opts, values) if profile and profile != "default": - values = config.get("profile:%s" % profile, {}) - cls._load_values_into_opts(opts, values) + try: + values = config["profile:%s" % profile] + cls._load_values_into_opts(opts, values) + except KeyError: + click.secho( + f"Warning: profile {profile} not found in config files {cls.config_files}", + fg="yellow", + ) + + existing_config_paths = { + path: os.path.exists(path) for path in cls.config_files + } + if not any(existing_config_paths.values()) and not cls.config_already_warned(): + click.secho( + "Warning: No config files found in search paths. Tried the following:", + fg="yellow", + ) + for tested_path, exists in existing_config_paths.items(): + if exists: + click.secho(f"{tested_path} - file exists", fg="green") + else: + click.secho(f"{tested_path} - file does not exist", fg="yellow") + click.secho( + "You may need to run `cloudsmith login` to authenticate and create a config file.", + fg="yellow", + ) return values @@ -206,6 +244,29 @@ class CredentialsReader(ConfigReader): config_searchpath = list(_CFG_SEARCH_PATHS) config_section_schemas = [CredentialsSchema.Default, CredentialsSchema.Profile] + @classmethod + def load_config(cls, opts, path=None, profile=None): + """ + Load a credentials configuration file into an options object. + We overload the load_config command in CredentialsReader as + credentials files have their own specific default functionality. + """ + if path and os.path.exists(path): + if os.path.isdir(path): + cls.config_searchpath.insert(0, path) + else: + cls.config_files.insert(0, path) + + config = cls.read_config() + values = config.get("default", {}) + cls._load_values_into_opts(opts, values) + + if profile and profile != "default": + values = config.get("profile:%s" % profile, {}) + cls._load_values_into_opts(opts, values) + + return values + class Options: """Options object that holds config for the application.""" diff --git a/cloudsmith_cli/cli/decorators.py b/cloudsmith_cli/cli/decorators.py index a8b41173..77947a52 100644 --- a/cloudsmith_cli/cli/decorators.py +++ b/cloudsmith_cli/cli/decorators.py @@ -5,6 +5,7 @@ import click from ..core.api.init import initialise_api as _initialise_api +from ..core.api.user import get_user_brief from . import config, utils, validators @@ -309,3 +310,27 @@ def call_print_rate_limit_info_with_opts(rate_info): return ctx.invoke(f, *args, **kwargs) return wrapper + + +def verify_authenticated(f): + """Verify that the user is authenticated and warn if not.""" + + @click.pass_context + @functools.wraps(f) + def wrapper(ctx, *args, **kwargs): + cloudsmith_host = kwargs["opts"].opts["api_config"].host + is_auth, _, _, _ = get_user_brief() + if not is_auth: + click.secho( + "Warning: You are not authenticated with the API. " + "Please verify your config files, API key and " + "run `cloudsmith login` if necessary to authenticate.", + fg="yellow", + ) + click.secho( + f"You're currently attemping to connect to Cloudsmith instance {cloudsmith_host}", + fg="yellow", + ) + return ctx.invoke(f, *args, **kwargs) + + return wrapper From 0616d584970624f415940f255ed8611345acdcf7 Mon Sep 17 00:00:00 2001 From: Kieran Manning Date: Mon, 28 Apr 2025 22:12:48 +0100 Subject: [PATCH 2/2] chore(eng-7893): removing duplicate warnings --- cloudsmith_cli/cli/commands/repos.py | 2 +- cloudsmith_cli/cli/config.py | 9 +++++---- cloudsmith_cli/cli/decorators.py | 17 +++++++++++++++-- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/cloudsmith_cli/cli/commands/repos.py b/cloudsmith_cli/cli/commands/repos.py index 79c12c9d..3f854336 100644 --- a/cloudsmith_cli/cli/commands/repos.py +++ b/cloudsmith_cli/cli/commands/repos.py @@ -60,7 +60,6 @@ def print_repositories(opts, data, page_info=None, show_list_info=True): @decorators.common_cli_output_options @decorators.common_api_auth_options @decorators.initialise_api -@decorators.verify_authenticated @click.pass_context def repositories(ctx, opts): # pylink: disable=unused-argument """ @@ -76,6 +75,7 @@ def repositories(ctx, opts): # pylink: disable=unused-argument @decorators.common_cli_output_options @decorators.common_api_auth_options @decorators.initialise_api +@decorators.verify_authenticated @click.argument( "owner_repo", metavar="OWNER/REPO", diff --git a/cloudsmith_cli/cli/config.py b/cloudsmith_cli/cli/config.py index 20c054d6..0c35aad8 100644 --- a/cloudsmith_cli/cli/config.py +++ b/cloudsmith_cli/cli/config.py @@ -179,10 +179,11 @@ def load_config(cls, opts, path=None, profile=None): values = config["profile:%s" % profile] cls._load_values_into_opts(opts, values) except KeyError: - click.secho( - f"Warning: profile {profile} not found in config files {cls.config_files}", - fg="yellow", - ) + if not cls.config_already_warned(): + click.secho( + f"Warning: profile {profile} not found in config files {cls.config_files}", + fg="yellow", + ) existing_config_paths = { path: os.path.exists(path) for path in cls.config_files diff --git a/cloudsmith_cli/cli/decorators.py b/cloudsmith_cli/cli/decorators.py index 77947a52..1ad908df 100644 --- a/cloudsmith_cli/cli/decorators.py +++ b/cloudsmith_cli/cli/decorators.py @@ -315,12 +315,20 @@ def call_print_rate_limit_info_with_opts(rate_info): def verify_authenticated(f): """Verify that the user is authenticated and warn if not.""" + @click.option( + "--no-warn", + default=False, + is_flag=True, + help="Don't warn on misconfiguration issues", + ) @click.pass_context @functools.wraps(f) def wrapper(ctx, *args, **kwargs): cloudsmith_host = kwargs["opts"].opts["api_config"].host + print(kwargs["opts"].opts["api_config"].__dict__) + no_warn = kwargs.pop("no_warn") is_auth, _, _, _ = get_user_brief() - if not is_auth: + if not is_auth and not no_warn: click.secho( "Warning: You are not authenticated with the API. " "Please verify your config files, API key and " @@ -328,9 +336,14 @@ def wrapper(ctx, *args, **kwargs): fg="yellow", ) click.secho( - f"You're currently attemping to connect to Cloudsmith instance {cloudsmith_host}", + f"You're currently attempting to connect to Cloudsmith instance {cloudsmith_host}", fg="yellow", ) + no_warn = True + + opts = config.get_or_create_options(ctx) + opts.no_warn = no_warn + kwargs["opts"] = opts return ctx.invoke(f, *args, **kwargs) return wrapper