diff --git a/CHANGELOG.md b/CHANGELOG.md index 22ecdf75..a8000b1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +### Deprecation Notice + +- The `--json` flag used in `cloudsmith auth` command will be removed in upcoming releases. Please migrate to `--output-format json` instead. + +### Fixed + +- Fixed JSON output for all commands + - Informational messages, warnings, and interactive prompts are now routed to stderr when `--output-format json` is active. + - Error messages are now formatted as structured JSON on stdout when JSON output is requested. + ## [1.10.1] - 2025-12-16 ### Fixed diff --git a/cloudsmith_cli/cli/command.py b/cloudsmith_cli/cli/command.py index cf75f8cf..3c1132cb 100644 --- a/cloudsmith_cli/cli/command.py +++ b/cloudsmith_cli/cli/command.py @@ -6,6 +6,46 @@ from click_didyoumean import DYMGroup +def _is_json_output_requested(exception): + """Determine if JSON output was requested, checking context and argv.""" + # Check context if available + ctx = getattr(exception, "ctx", None) + if ctx and ctx.params: + fmt = ctx.params.get("output") + if fmt in ("json", "pretty_json"): + return True + + # Fallback: check sys.argv for output format flags + import sys + + argv = sys.argv + + if "--output-format=json" in argv or "--output-format=pretty_json" in argv: + return True + + for idx, arg in enumerate(argv): + if arg in ("-F", "--output-format") and idx + 1 < len(argv): + if argv[idx + 1] in ("json", "pretty_json"): + return True + + return False + + +def _format_click_exception_as_json(exception): + """Format a ClickException as a JSON error dict.""" + return { + "detail": exception.format_message(), + "meta": { + "code": exception.exit_code, + "description": "Usage Error", + }, + "help": { + "context": "Invalid usage", + "hint": "Check your command arguments/flags.", + }, + } + + class AliasGroup(DYMGroup): """A command group with DYM and alias support.""" @@ -92,3 +132,29 @@ def decorator(f): def format_commands(self, ctx, formatter): ctx.showing_help = True return super().format_commands(ctx, formatter) + + def main(self, *args, **kwargs): + """Override main to intercept exceptions and format as JSON if requested.""" + import sys + + original_standalone_mode = kwargs.get("standalone_mode", True) + kwargs["standalone_mode"] = False + + try: + return super().main(*args, **kwargs) + except click.exceptions.Abort: + if not original_standalone_mode: + raise + click.echo("Aborted!", err=True) + sys.exit(1) + except click.exceptions.ClickException as e: + if _is_json_output_requested(e): + import json + + click.echo(json.dumps(_format_click_exception_as_json(e), indent=4)) + sys.exit(e.exit_code) + + if not original_standalone_mode: + raise + e.show() + sys.exit(e.exit_code) diff --git a/cloudsmith_cli/cli/commands/auth.py b/cloudsmith_cli/cli/commands/auth.py index 984e455f..cc90b400 100644 --- a/cloudsmith_cli/cli/commands/auth.py +++ b/cloudsmith_cli/cli/commands/auth.py @@ -4,7 +4,7 @@ import click -from .. import decorators, validators +from .. import decorators, utils, validators from ..exceptions import handle_api_exceptions from ..saml import create_configured_session, get_idp_url from ..webserver import AuthenticationWebRequestHandler, AuthenticationWebServer @@ -22,14 +22,15 @@ def _perform_saml_authentication(opts, owner, enable_token_creation=False, json= api_host = opts.api_config.host idp_url = get_idp_url(api_host, owner, session=session) - if not json: - click.echo( - f"Opening your organization's SAML IDP URL in your browser: {click.style(idp_url, bold=True)}" - ) - click.echo() + + click.echo( + f"Opening your organization's SAML IDP URL in your browser: {click.style(idp_url, bold=True)}", + err=json, + ) + click.echo(err=json) webbrowser.open(idp_url) - if not json: - click.echo("Starting webserver to begin authentication ... ") + + click.echo("Starting webserver to begin authentication ... ", err=json) auth_server = AuthenticationWebServer( (AUTH_SERVER_HOST, AUTH_SERVER_PORT), @@ -86,13 +87,25 @@ def _perform_saml_authentication(opts, owner, enable_token_creation=False, json= @click.pass_context def authenticate(ctx, opts, owner, token, force, save_config, json): """Authenticate to Cloudsmith using the org's SAML setup.""" - owner = owner[0].strip("'[]'") + json = json or utils.should_use_stderr(opts) + # If using json output, we redirect info messages to stderr + use_stderr = json - if not json: - click.echo( - f"Beginning authentication for the {click.style(owner, bold=True)} org ... " + if json and not utils.should_use_stderr(opts): + click.secho( + "DEPRECATION WARNING: The `--json` flag is deprecated and will be removed in a future release. " + "Please use `--output-format json` instead.", + fg="yellow", + err=True, ) + owner = owner[0].strip("'[]'") + + click.echo( + f"Beginning authentication for the {click.style(owner, bold=True)} org ... ", + err=use_stderr, + ) + context_message = "Failed to authenticate via SSO!" with handle_api_exceptions(ctx, opts=opts, context_msg=context_message): _perform_saml_authentication( diff --git a/cloudsmith_cli/cli/commands/check.py b/cloudsmith_cli/cli/commands/check.py index de25a84b..ea0772b5 100644 --- a/cloudsmith_cli/cli/commands/check.py +++ b/cloudsmith_cli/cli/commands/check.py @@ -29,14 +29,18 @@ def check(ctx, opts): # pylint: disable=unused-argument @click.pass_context def rates(ctx, opts): """Check current API rate limits.""" - click.echo("Retrieving rate limits ... ", nl=False) + use_stderr = utils.should_use_stderr(opts) + click.echo("Retrieving rate limits ... ", nl=False, err=use_stderr) context_msg = "Failed to retrieve status!" with handle_api_exceptions(ctx, opts=opts, context_msg=context_msg): with maybe_spinner(opts): resources_limits = get_rate_limits() - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) + + if utils.maybe_print_as_json(opts, resources_limits): + return headers = ["Resource", "Throttled", "Remaining", "Interval (Seconds)", "Reset"] @@ -77,17 +81,27 @@ def rates(ctx, opts): @click.pass_context def service(ctx, opts): """Check the status of the Cloudsmith service.""" - click.echo("Retrieving service status ... ", nl=False) + use_stderr = utils.should_use_stderr(opts) + click.echo("Retrieving service status ... ", nl=False, err=use_stderr) context_msg = "Failed to retrieve status!" with handle_api_exceptions(ctx, opts=opts, context_msg=context_msg): with maybe_spinner(opts): status, version = get_status(with_version=True) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) config = cloudsmith_api.Configuration() + data = { + "endpoint": config.host, + "status": status, + "version": version, + } + + if utils.maybe_print_as_json(opts, data): + return + click.echo() click.echo(f"The service endpoint is: {click.style(config.host, bold=True)}") click.echo(f"The service status is: {click.style(status, bold=True)}") diff --git a/cloudsmith_cli/cli/commands/copy.py b/cloudsmith_cli/cli/commands/copy.py index 619443f3..2f52d3c7 100644 --- a/cloudsmith_cli/cli/commands/copy.py +++ b/cloudsmith_cli/cli/commands/copy.py @@ -3,7 +3,7 @@ import click from ...core.api.packages import copy_package -from .. import decorators, validators +from .. import decorators, utils, validators from ..exceptions import handle_api_exceptions from ..utils import maybe_spinner from .main import main @@ -56,6 +56,8 @@ def copy( """ owner, source, slug = owner_repo_package + use_stderr = utils.should_use_stderr(opts) + click.echo( "Copying %(slug)s package from %(source)s to %(dest)s ... " % { @@ -64,6 +66,7 @@ def copy( "dest": click.style(destination, bold=True), }, nl=False, + err=use_stderr, ) context_msg = "Failed to copy package!" @@ -75,9 +78,10 @@ def copy( owner=owner, repo=source, identifier=slug, destination=destination ) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) if no_wait_for_sync: + utils.maybe_print_status_json(opts, {"slug": new_slug, "status": "OK"}) return wait_for_package_sync( @@ -90,3 +94,5 @@ def copy( skip_errors=skip_errors, attempts=sync_attempts, ) + + utils.maybe_print_status_json(opts, {"slug": new_slug, "status": "OK"}) diff --git a/cloudsmith_cli/cli/commands/delete.py b/cloudsmith_cli/cli/commands/delete.py index 245c1feb..af8b127c 100644 --- a/cloudsmith_cli/cli/commands/delete.py +++ b/cloudsmith_cli/cli/commands/delete.py @@ -45,12 +45,16 @@ def delete(ctx, opts, owner_repo_package, yes): "package": click.style(slug, bold=True), } + use_stderr = utils.should_use_stderr(opts) + prompt = "delete the %(package)s from %(owner)s/%(repo)s" % delete_args - if not utils.confirm_operation(prompt, assume_yes=yes): + if not utils.confirm_operation(prompt, assume_yes=yes, err=use_stderr): return click.echo( - "Deleting %(package)s from %(owner)s/%(repo)s ... " % delete_args, nl=False + "Deleting %(package)s from %(owner)s/%(repo)s ... " % delete_args, + nl=False, + err=use_stderr, ) context_msg = "Failed to delete the package!" @@ -58,4 +62,6 @@ def delete(ctx, opts, owner_repo_package, yes): with maybe_spinner(opts): delete_package(owner=owner, repo=repo, identifier=slug) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) + + utils.maybe_print_status_json(opts, {"slug": slug, "status": "OK"}) diff --git a/cloudsmith_cli/cli/commands/dependencies.py b/cloudsmith_cli/cli/commands/dependencies.py index b11dcb71..98187250 100644 --- a/cloudsmith_cli/cli/commands/dependencies.py +++ b/cloudsmith_cli/cli/commands/dependencies.py @@ -50,7 +50,7 @@ def list_dependencies(ctx, opts, owner_repo_package): owner, repo, identifier = owner_repo_package # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) click.echo( "Getting direct (non-transitive) dependencies of %(package)s in " diff --git a/cloudsmith_cli/cli/commands/download.py b/cloudsmith_cli/cli/commands/download.py index 65ced8a9..8d4176e3 100644 --- a/cloudsmith_cli/cli/commands/download.py +++ b/cloudsmith_cli/cli/commands/download.py @@ -12,7 +12,7 @@ resolve_package, stream_download, ) -from .. import decorators, validators +from .. import decorators, utils, validators from ..exceptions import handle_api_exceptions from ..utils import maybe_spinner from .main import main @@ -124,7 +124,7 @@ def download( # noqa: C901 owner, repo = owner_repo # Use stderr for messages if output is JSON - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) if not use_stderr: click.echo( @@ -213,11 +213,13 @@ def download( # noqa: C901 return # Download all files - click.echo(f"\nDownloading {len(files_to_download)} files to: {output_dir}") - click.echo() + if not use_stderr: + click.echo(f"\nDownloading {len(files_to_download)} files to: {output_dir}") + click.echo() success_count = 0 failed_files = [] + downloaded_files = [] for idx, file_info in enumerate(files_to_download, 1): filename = file_info["filename"] @@ -232,6 +234,12 @@ def download( # noqa: C901 f"[{idx}/{len(files_to_download)}] [{tag}] {filename}{primary_marker} ...", nl=False, ) + else: + click.echo( + f"[{idx}/{len(files_to_download)}] [{tag}] {filename}{primary_marker} ...", + nl=False, + err=True, + ) try: context_msg = f"Failed to download {filename}!" @@ -246,11 +254,56 @@ def download( # noqa: C901 ) if not use_stderr: click.secho(" OK", fg="green") + else: + click.echo(" OK", err=True) success_count += 1 + downloaded_files.append( + { + "filename": filename, + "path": output_path, + "tag": tag, + "is_primary": file_info.get("is_primary", False), + "size": file_info.get("size", 0), + "status": "OK", + } + ) except Exception as e: # pylint: disable=broad-except if not use_stderr: click.secho(" FAILED", fg="red") + else: + click.echo(" FAILED", err=True) failed_files.append((filename, str(e))) + downloaded_files.append( + { + "filename": filename, + "path": output_path, + "tag": tag, + "is_primary": file_info.get("is_primary", False), + "size": file_info.get("size", 0), + "status": "FAILED", + "error": str(e), + } + ) + + # Build JSON output for all-files download + json_output = { + "package": { + "name": package.get("name"), + "version": package.get("version"), + "format": package.get("format"), + "slug": package.get("slug"), + }, + "output_directory": output_dir, + "files": downloaded_files, + "summary": { + "total": len(files_to_download), + "success": success_count, + "failed": len(failed_files), + }, + } + + if utils.maybe_print_as_json(opts, json_output): + return click.echo() if success_count == len(files_to_download): @@ -321,12 +374,28 @@ def download( # noqa: C901 session=session, headers=auth_headers, overwrite=overwrite, - quiet=opts.output != "pretty", + quiet=utils.should_use_stderr(opts), ) - if opts.output == "pretty": - click.echo() - click.secho("Download completed successfully!", fg="green") + # Build JSON output for single-file download + json_output = { + "package": { + "name": package.get("name"), + "version": package.get("version"), + "format": package.get("format"), + "slug": package.get("slug"), + }, + "filename": os.path.basename(outfile), + "path": outfile, + "size": package.get("size", 0), + "status": "OK", + } + + if utils.maybe_print_as_json(opts, json_output): + return + + click.echo() + click.secho("Download completed successfully!", fg="green") def _get_extension_for_format(pkg_format: str) -> str: diff --git a/cloudsmith_cli/cli/commands/entitlements.py b/cloudsmith_cli/cli/commands/entitlements.py index 72d18938..aa465f6f 100644 --- a/cloudsmith_cli/cli/commands/entitlements.py +++ b/cloudsmith_cli/cli/commands/entitlements.py @@ -89,7 +89,7 @@ def list_entitlements(ctx, opts, owner_repo, page, page_size, show_tokens, page_ owner, repo = owner_repo # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) click.echo( "Getting list of entitlements for the %(repository)s " @@ -323,7 +323,7 @@ def create(ctx, opts, owner_repo, show_tokens, name, token): owner, repo = owner_repo # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) click.secho( "Creating %(name)s entitlement for the %(repository)s " @@ -391,13 +391,17 @@ def delete(ctx, opts, owner_repo_identifier, yes): "delete the %(identifier)s entitlement from the %(repository)s " "repository" % delete_args ) - if not utils.confirm_operation(prompt, assume_yes=yes): + + use_stderr = utils.should_use_stderr(opts) + + if not utils.confirm_operation(prompt, assume_yes=yes, err=use_stderr): return click.secho( "Deleting %(identifier)s entitlement from the %(repository)s " "repository ... " % delete_args, nl=False, + err=use_stderr, ) context_msg = "Failed to delete the entitlement!" @@ -454,7 +458,7 @@ def update(ctx, opts, owner_repo_identifier, show_tokens, name, token): owner, repo, identifier = owner_repo_identifier # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) click.secho( "Updating %(identifier)s entitlement for the %(repository)s " @@ -527,7 +531,7 @@ def refresh(ctx, opts, owner_repo_identifier, show_tokens, yes): } # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) prompt = ( "refresh the %(identifier)s entitlement for the %(repository)s " @@ -603,7 +607,7 @@ def sync(ctx, opts, owner_repo, show_tokens, source, yes): } # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) if not yes: click.secho( @@ -768,7 +772,7 @@ def restrict( owner, repo, identifier = owner_repo_identifier # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) click.secho( "Updating %(identifier)s entitlement for the %(repository)s " diff --git a/cloudsmith_cli/cli/commands/list_.py b/cloudsmith_cli/cli/commands/list_.py index 4424cfb4..f958ac0e 100644 --- a/cloudsmith_cli/cli/commands/list_.py +++ b/cloudsmith_cli/cli/commands/list_.py @@ -53,7 +53,7 @@ def dependencies_(*args, **kwargs): # pylint: disable=missing-docstring def distros(ctx, opts, package_format): """List available distributions.""" # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) if not use_stderr: click.echo("Getting list of distributions ... ", nl=False, err=use_stderr) @@ -210,7 +210,7 @@ def packages(ctx, opts, owner_repo, page, page_size, query, sort, page_all): owner, repo = owner_repo # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) if not use_stderr: click.echo("Getting list of packages ... ", nl=False, err=use_stderr) diff --git a/cloudsmith_cli/cli/commands/login.py b/cloudsmith_cli/cli/commands/login.py index da885ac1..43840dc2 100644 --- a/cloudsmith_cli/cli/commands/login.py +++ b/cloudsmith_cli/cli/commands/login.py @@ -6,7 +6,7 @@ from ...core.api.exceptions import TwoFactorRequiredException from ...core.api.user import get_user_token from ...core.config import create_config_files, new_config_messaging -from .. import decorators +from .. import decorators, utils from ..exceptions import handle_api_exceptions from ..utils import maybe_spinner from .main import main @@ -37,10 +37,12 @@ def validate_login(ctx, param, value): @click.pass_context def login(ctx, opts, login, password): # pylint: disable=redefined-outer-name """Retrieve your API authentication token/key via login.""" + use_stderr = utils.should_use_stderr(opts) click.echo( "Retrieving API token for %(login)s ... " % {"login": click.style(login, bold=True)}, nl=False, + err=use_stderr, ) context_msg = "Failed to retrieve the API token!" @@ -49,14 +51,17 @@ def login(ctx, opts, login, password): # pylint: disable=redefined-outer-name with maybe_spinner(opts): api_key = get_user_token(login=login, password=password) except TwoFactorRequiredException as e: - click.echo("\r\033[K", nl=False) - click.echo("Two-factor authentication is required.") + click.echo("\r\033[K", nl=False, err=use_stderr) + click.echo("Two-factor authentication is required.", err=use_stderr) - totp_token = click.prompt("Enter your two-factor authentication code", type=str) + totp_token = click.prompt( + "Enter your two-factor authentication code", type=str, err=use_stderr + ) click.echo( "Verifying two-factor code for %(login)s ... " % {"login": click.style(login, bold=True)}, nl=False, + err=use_stderr, ) try: @@ -69,23 +74,26 @@ def login(ctx, opts, login, password): # pylint: disable=redefined-outer-name two_factor_token=e.two_factor_token, ) except cloudsmith_api.rest.ApiException: - click.echo("\r\033[K", nl=False) + click.echo("\r\033[K", nl=False, err=use_stderr) click.secho( - "Authentication failed: The entered TOTP token is not valid.", fg="red" + "Authentication failed: The entered TOTP token is not valid.", + fg="red", + err=use_stderr, ) ctx.exit(1) except cloudsmith_api.rest.ApiException as e: - click.echo("\r\033[K", nl=False) - click.secho(f"Authentication failed: {str(e)}", fg="red") + click.echo("\r\033[K", nl=False, err=use_stderr) + click.secho(f"Authentication failed: {str(e)}", fg="red", err=use_stderr) ctx.exit(1) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) - click.echo( - "Your API key/token is: %(token)s" - % {"token": click.style(api_key, fg="magenta")} - ) + if not utils.maybe_print_as_json(opts, {"token": api_key, "login": login}): + click.echo( + "Your API key/token is: %(token)s" + % {"token": click.style(api_key, fg="magenta")} + ) create, has_errors = create_config_files(ctx, opts, api_key=api_key) new_config_messaging(has_errors, opts, create, api_key=api_key) diff --git a/cloudsmith_cli/cli/commands/metrics/entitlements.py b/cloudsmith_cli/cli/commands/metrics/entitlements.py index 0b897149..7f627dc3 100644 --- a/cloudsmith_cli/cli/commands/metrics/entitlements.py +++ b/cloudsmith_cli/cli/commands/metrics/entitlements.py @@ -109,8 +109,8 @@ def usage(ctx, opts, owner_repo, tokens, start, finish): If REPO isn't specified, all repositories will be included from the OWNER namespace. """ - # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + + use_stderr = utils.should_use_stderr(opts) click.echo("Getting usage metrics ... ", nl=False, err=use_stderr) diff --git a/cloudsmith_cli/cli/commands/metrics/packages.py b/cloudsmith_cli/cli/commands/metrics/packages.py index eabb11ae..9fa03264 100644 --- a/cloudsmith_cli/cli/commands/metrics/packages.py +++ b/cloudsmith_cli/cli/commands/metrics/packages.py @@ -106,7 +106,7 @@ def usage(ctx, opts, owner_repo, packages, start, finish): metrics for that namespace/repository combination. """ # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) click.echo("Getting usage metrics ... ", nl=False, err=use_stderr) diff --git a/cloudsmith_cli/cli/commands/move.py b/cloudsmith_cli/cli/commands/move.py index 0607b523..94822134 100644 --- a/cloudsmith_cli/cli/commands/move.py +++ b/cloudsmith_cli/cli/commands/move.py @@ -70,12 +70,16 @@ def move( "dest": click.style(destination, bold=True), } + use_stderr = utils.should_use_stderr(opts) + prompt = "move the %(slug)s from %(source)s to %(dest)s" % move_args - if not utils.confirm_operation(prompt, assume_yes=yes): + if not utils.confirm_operation(prompt, assume_yes=yes, err=use_stderr): return click.echo( - "Moving %(slug)s package from %(source)s to %(dest)s ... " % move_args, nl=False + "Moving %(slug)s package from %(source)s to %(dest)s ... " % move_args, + nl=False, + err=use_stderr, ) context_msg = "Failed to move package!" @@ -87,9 +91,10 @@ def move( owner=owner, repo=source, identifier=slug, destination=destination ) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) if no_wait_for_sync: + utils.maybe_print_status_json(opts, {"slug": new_slug, "status": "OK"}) return wait_for_package_sync( @@ -102,3 +107,5 @@ def move( skip_errors=skip_errors, attempts=sync_attempts, ) + + utils.maybe_print_status_json(opts, {"slug": new_slug, "status": "OK"}) diff --git a/cloudsmith_cli/cli/commands/policy/deny.py b/cloudsmith_cli/cli/commands/policy/deny.py index 10a2c9ef..b555b83a 100644 --- a/cloudsmith_cli/cli/commands/policy/deny.py +++ b/cloudsmith_cli/cli/commands/policy/deny.py @@ -63,7 +63,7 @@ def deny_policy(*args, **kwargs): @click.pass_context def list_deny_policies(ctx, opts, owner, page, page_size, page_all): """List deny policies for an organization.""" - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) click.echo("Getting deny policies ... ", nl=False, err=use_stderr) context_msg = "Failed to get deny policies!" @@ -104,7 +104,7 @@ def create_deny_policy(ctx, opts, owner, policy_config_file): """Create a deny policy for an organization.""" import json - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) policy_config = json.load(policy_config_file) policy_name = policy_config.get("name") @@ -146,7 +146,7 @@ def create_deny_policy(ctx, opts, owner, policy_config_file): @click.pass_context def get_deny_policy(ctx, opts, owner, identifier): """Get a deny policy for an organization.""" - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) click.echo("Getting deny policy ... ", nl=False, err=use_stderr) context_msg = "Failed to get deny policy!" @@ -175,7 +175,7 @@ def update_deny_policy(ctx, opts, owner, identifier, policy_config_file): """Update a deny policy for an organization.""" import json - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) policy_config = json.load(policy_config_file) click.secho( @@ -230,12 +230,14 @@ def delete_deny_policy(ctx, opts, owner, identifier, yes): % delete_args ) - if not utils.confirm_operation(prompt, assume_yes=yes): + use_stderr = utils.should_use_stderr(opts) + if not utils.confirm_operation(prompt, assume_yes=yes, err=use_stderr): return click.secho( "Deleting %(identifier)s from the %(namespace)s namespace ... " % delete_args, nl=False, + err=use_stderr, ) context_msg = "Failed to delete the deny policy!" @@ -243,4 +245,4 @@ def delete_deny_policy(ctx, opts, owner, identifier, yes): with maybe_spinner(opts): orgs.delete_deny_policy(owner=owner, slug_perm=identifier) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) diff --git a/cloudsmith_cli/cli/commands/policy/license.py b/cloudsmith_cli/cli/commands/policy/license.py index 45a3df60..3e1063eb 100644 --- a/cloudsmith_cli/cli/commands/policy/license.py +++ b/cloudsmith_cli/cli/commands/policy/license.py @@ -100,7 +100,7 @@ def ls(ctx, opts, owner, page, page_size, page_all): owner = owner[0] # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) click.echo("Getting license policies ... ", nl=False, err=use_stderr) @@ -316,13 +316,15 @@ def delete(ctx, opts, owner, identifier, yes): "delete the %(slug_perm)s license policy from the %(namespace)s namespace" % delete_args ) + use_stderr = utils.should_use_stderr(opts) - if not utils.confirm_operation(prompt, assume_yes=yes): + if not utils.confirm_operation(prompt, assume_yes=yes, err=use_stderr): return click.secho( "Deleting %(slug_perm)s from the %(namespace)s namespace ... " % delete_args, nl=False, + err=use_stderr, ) context_msg = "Failed to delete the license policy!" @@ -330,4 +332,4 @@ def delete(ctx, opts, owner, identifier, yes): with maybe_spinner(opts): api.delete_license_policy(owner=owner, slug_perm=identifier) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) diff --git a/cloudsmith_cli/cli/commands/policy/vulnerability.py b/cloudsmith_cli/cli/commands/policy/vulnerability.py index 81c56895..ff72b676 100644 --- a/cloudsmith_cli/cli/commands/policy/vulnerability.py +++ b/cloudsmith_cli/cli/commands/policy/vulnerability.py @@ -90,7 +90,7 @@ def ls(ctx, opts, owner, page, page_size, page_all): owner = owner[0] # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) click.echo("Getting vulnerability policies ... ", nl=False, err=use_stderr) @@ -303,12 +303,15 @@ def delete(ctx, opts, owner, identifier, yes): % delete_args ) - if not utils.confirm_operation(prompt, assume_yes=yes): + use_stderr = utils.should_use_stderr(opts) + + if not utils.confirm_operation(prompt, assume_yes=yes, err=use_stderr): return click.secho( "Deleting %(slug_perm)s from the %(namespace)s namespace ... " % delete_args, nl=False, + err=use_stderr, ) context_msg = "Failed to delete the vulnerability policy!" @@ -316,4 +319,4 @@ def delete(ctx, opts, owner, identifier, yes): with maybe_spinner(opts): api.delete_vulnerability_policy(owner=owner, slug_perm=identifier) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) diff --git a/cloudsmith_cli/cli/commands/push.py b/cloudsmith_cli/cli/commands/push.py index c5e74125..2cc3d663 100644 --- a/cloudsmith_cli/cli/commands/push.py +++ b/cloudsmith_cli/cli/commands/push.py @@ -7,7 +7,7 @@ import click -from ...core import utils +from ...core import utils as core_utils from ...core.api.exceptions import ApiException from ...core.api.files import ( CHUNK_SIZE, @@ -22,7 +22,7 @@ get_package_status, validate_create_package as api_validate_create_package, ) -from .. import command, decorators, validators +from .. import command, decorators, utils, validators from ..exceptions import handle_api_exceptions from ..types import ExpandPath from ..utils import maybe_spinner @@ -34,10 +34,13 @@ def validate_upload_file(ctx, opts, owner, repo, filepath, skip_errors): filename = click.format_filename(filepath) basename = os.path.basename(filename) + use_stderr = utils.should_use_stderr(opts) + click.echo( "Checking %(filename)s file upload parameters ... " % {"filename": click.style(basename, bold=True)}, nl=False, + err=use_stderr, ) context_msg = "Failed to validate upload parameters!" @@ -49,7 +52,7 @@ def validate_upload_file(ctx, opts, owner, repo, filepath, skip_errors): owner=owner, repo=repo, filepath=filename ) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) return md5_checksum @@ -59,14 +62,17 @@ def upload_file(ctx, opts, owner, repo, filepath, skip_errors, md5_checksum): filename = click.format_filename(filepath) basename = os.path.basename(filename) - filesize = utils.get_file_size(filepath=filename) + filesize = core_utils.get_file_size(filepath=filename) projected_chunks = math.floor(filesize / CHUNK_SIZE) + 1 is_multi_part_upload = projected_chunks > 1 + use_stderr = utils.should_use_stderr(opts) + click.echo( "Requesting file upload for %(filename)s ... " % {"filename": click.style(basename, bold=True)}, nl=False, + err=use_stderr, ) context_msg = "Failed to request file upload!" @@ -82,51 +88,69 @@ def upload_file(ctx, opts, owner, repo, filepath, skip_errors, md5_checksum): is_multi_part_upload=is_multi_part_upload, ) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) context_msg = "Failed to upload file!" with handle_api_exceptions(ctx, opts=opts, context_msg=context_msg): label = f"Uploading {click.style(basename, bold=True)}:" if not is_multi_part_upload: - # We can upload the whole file in one go. - with click.progressbar( - length=filesize, - label=label, - fill_char=click.style("#", fg="green"), - empty_char=click.style("-", fg="red"), - ) as pb: - - def progress_callback(monitor): - pb.update(monitor.bytes_read) - + if use_stderr: api_upload_file( upload_url=upload_url, upload_fields=upload_fields, filepath=filename, - callback=progress_callback, ) + else: + # We can upload the whole file in one go. + with click.progressbar( + length=filesize, + label=label, + fill_char=click.style("#", fg="green"), + empty_char=click.style("-", fg="red"), + ) as pb: + + def progress_callback(monitor): + pb.update(monitor.bytes_read) + + api_upload_file( + upload_url=upload_url, + upload_fields=upload_fields, + filepath=filename, + callback=progress_callback, + ) else: - # The file is sufficiently large that we need to upload in chunks. - with click.progressbar( - length=projected_chunks, - label=label, - fill_char=click.style("#", fg="green"), - empty_char=click.style("-", fg="red"), - ) as pb: - - def progress_callback(): - pb.update(1) - + if use_stderr: multi_part_upload_file( opts=opts, upload_url=upload_url, owner=owner, repo=repo, filepath=filename, - callback=progress_callback, upload_id=identifier, + callback=lambda: None, ) + else: + # The file is sufficiently large that we need to upload in chunks. + with click.progressbar( + length=projected_chunks, + label=label, + fill_char=click.style("#", fg="green"), + empty_char=click.style("-", fg="red"), + ) as pb: + + def progress_callback(): + pb.update(1) + + multi_part_upload_file( + opts=opts, + upload_url=upload_url, + owner=owner, + repo=repo, + filepath=filename, + callback=progress_callback, + upload_id=identifier, + ) return identifier @@ -135,10 +159,13 @@ def validate_create_package( ctx, opts, owner, repo, package_type, skip_errors, **kwargs ): """Check new package parameters via the API.""" + use_stderr = utils.should_use_stderr(opts) + click.echo( "Checking %(package_type)s package upload parameters ... " % {"package_type": click.style(package_type, bold=True)}, nl=False, + err=use_stderr, ) context_msg = "Failed to validate upload parameters!" @@ -150,16 +177,19 @@ def validate_create_package( package_format=package_type, owner=owner, repo=repo, **kwargs ) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) return True def create_package(ctx, opts, owner, repo, package_type, skip_errors, **kwargs): """Create a new package via the API.""" + use_stderr = utils.should_use_stderr(opts) + click.echo( "Creating a new %(package_type)s package ... " % {"package_type": click.style(package_type, bold=True)}, nl=False, + err=use_stderr, ) context_msg = "Failed to create package!" @@ -171,7 +201,7 @@ def create_package(ctx, opts, owner, repo, package_type, skip_errors, **kwargs): package_format=package_type, owner=owner, repo=repo, **kwargs ) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) click.echo( "Created: %(owner)s/%(repo)s/%(slug)s (%(slug_perm)s)" @@ -180,7 +210,8 @@ def create_package(ctx, opts, owner, repo, package_type, skip_errors, **kwargs): "repo": click.style(repo, fg="magenta"), "slug": click.style(slug, fg="green"), "slug_perm": click.style(slug_perm, bold=True), - } + }, + err=use_stderr, ) return slug_perm, slug @@ -191,8 +222,10 @@ def wait_for_package_sync( ): """Wait for a package to synchronise (or fail).""" # pylint: disable=too-many-locals + use_stderr = utils.should_use_stderr(opts) + attempts -= 1 - click.echo() + click.echo(err=use_stderr) label = f"Synchronising {click.style(slug, fg='green')}:" status_str = "Waiting" @@ -218,46 +251,64 @@ def display_status(current): total_wait_interval = max(1.0, wait_interval) first = True - with click.progressbar( - length=left, - label=label, - fill_char=click.style("#", fg="green"), - empty_char=click.style("-", fg="red"), - item_show_func=display_status, - ) as pb: + if use_stderr: + # When using stderr for logs, avoid an interactive progress bar and just poll for status. while True: res = get_package_status(owner, repo, slug) - ok, failed, progress, status_str, stage_str, reason = res - progress = max(1, progress) - delta = progress - last_progress - pb.update(delta) - if delta > 0: - last_progress = progress - left -= delta + ok, failed, _, _, _, _ = res if ok or failed: break - if first: - first = False - else: - # Sleep, but only after the first status call + + # Sleep if we are going to loop again + if not first: time.sleep(total_wait_interval) total_wait_interval = min( 300.0, total_wait_interval + wait_interval ) + first = False - if left > 0: - pb.update(left) + else: + with click.progressbar( + length=left, + label=label, + fill_char=click.style("#", fg="green"), + empty_char=click.style("-", fg="red"), + item_show_func=display_status, + ) as pb: + while True: + res = get_package_status(owner, repo, slug) + ok, failed, progress, status_str, stage_str, reason = res + progress = max(1, progress) + delta = progress - last_progress + pb.update(delta) + if delta > 0: + last_progress = progress + left -= delta + if ok or failed: + break + if first: + first = False + else: + # Sleep, but only after the first status call + time.sleep(total_wait_interval) + total_wait_interval = min( + 300.0, total_wait_interval + wait_interval + ) + + if left > 0: + pb.update(left) end = datetime.now() seconds = (end - start).total_seconds() - click.echo() + click.echo(err=use_stderr) if ok: click.secho( "Package synchronised successfully in %(seconds)s second(s)!" % {"seconds": click.style(str(seconds), bold=True)}, fg="green", + err=use_stderr, ) return @@ -268,21 +319,25 @@ def display_status(current): "stage": click.style(stage_str or "Unknown", fg="yellow"), }, fg="red", + err=use_stderr, ) if reason: click.secho( f"Reason given: {click.style(reason, fg='yellow')}", fg="red", + err=use_stderr, ) # pylint: disable=fixme # FIXME: The API should communicate "no retry" fails if "package should be deleted" in reason and attempts > 1: click.secho( - "This is not recoverable, so stopping further attempts!", fg="red" + "This is not recoverable, so stopping further attempts!", + fg="red", + err=use_stderr, ) - click.echo() + click.echo(err=use_stderr) attempts = 0 if attempts + 1 > 0: @@ -292,9 +347,10 @@ def display_status(current): % { "left": click.style(str(attempts), bold=True), "action": "trying again" if attempts > 0 else "giving up", - } + }, + err=use_stderr, ) - click.echo() + click.echo(err=use_stderr) if attempts > 0: from .resync import resync_package @@ -418,7 +474,7 @@ def upload_files_and_create_package( ] # 4. Create the package with package files and additional arguments - _, slug = create_package( + slug_perm, slug = create_package( ctx=ctx, opts=opts, owner=owner, @@ -429,7 +485,7 @@ def upload_files_and_create_package( ) if no_wait_for_sync: - return + return slug_perm, slug # 5. (optionally) Wait for the package to synchronise wait_for_package_sync( @@ -443,6 +499,8 @@ def upload_files_and_create_package( attempts=sync_attempts, ) + return slug_perm, slug + def create_push_handlers(): """Create a handler for upload per package format.""" @@ -530,6 +588,7 @@ def create_push_handlers(): @click.pass_context def push_handler(ctx, *args, **kwargs): """Handle upload for a specific package format.""" + opts = kwargs.get("opts") parameters = context.get(ctx.info_name) kwargs["package_type"] = ctx.info_name @@ -543,16 +602,39 @@ def push_handler(ctx, *args, **kwargs): if not isinstance(package_files, tuple): package_files = (package_files,) + results = [] for package_file in package_files: kwargs["package_file"] = package_file try: - click.echo() - upload_files_and_create_package(ctx, *args, **kwargs) + click.echo(err=utils.should_use_stderr(opts)) + res = upload_files_and_create_package(ctx, *args, **kwargs) + if res: + results.append(res) except ApiException: - click.secho("Skipping error and moving on.", fg="yellow") + click.secho( + "Skipping error and moving on.", + fg="yellow", + err=utils.should_use_stderr(opts), + ) - click.echo() + click.echo(err=utils.should_use_stderr(opts)) + + if utils.should_use_stderr(opts): + data = [] + for slug_perm, slug in results: + data.append( + { + "slug_perm": slug_perm, + "slug": slug, + "status": "OK", # Assuming success if we got here + } + ) + + if len(data) == 1: + utils.maybe_print_as_json(opts, data[0]) + else: + utils.maybe_print_as_json(opts, data) # Add any additional arguments for k, info in kwargs.items(): diff --git a/cloudsmith_cli/cli/commands/quarantine.py b/cloudsmith_cli/cli/commands/quarantine.py index 028c3ed9..07024f55 100644 --- a/cloudsmith_cli/cli/commands/quarantine.py +++ b/cloudsmith_cli/cli/commands/quarantine.py @@ -5,7 +5,7 @@ import click from ...core.api import packages as api -from .. import command, decorators, validators +from .. import command, decorators, utils, validators from ..exceptions import handle_api_exceptions from ..utils import maybe_spinner from .main import main @@ -69,8 +69,7 @@ def add_quarantine(ctx, opts, owner_repo_package, page, page_size, page_all): """ owner, repo, slug = owner_repo_package - # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) click.echo( "Adding %(repository)s/%(package_slug)s to quarantine... " @@ -89,6 +88,8 @@ def add_quarantine(ctx, opts, owner_repo_package, page, page_size, page_all): click.secho("OK", fg="green", err=use_stderr) + utils.maybe_print_status_json(opts, {"slug": slug, "status": "OK"}) + @quarantine.command(name="add") @common_quarantine_options @@ -117,7 +118,7 @@ def remove_quarantine(ctx, opts, owner_repo_package, page, page_size, page_all): owner, repo, slug = owner_repo_package # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) click.echo( "Removing %(repository)s/%(package_slug)s from quarantine... " @@ -136,6 +137,8 @@ def remove_quarantine(ctx, opts, owner_repo_package, page, page_size, page_all): click.secho("OK", fg="green", err=use_stderr) + utils.maybe_print_status_json(opts, {"slug": slug, "status": "OK"}) + @quarantine.command(name="remove", aliases=["rm", "restore"]) @common_quarantine_options diff --git a/cloudsmith_cli/cli/commands/quota/history.py b/cloudsmith_cli/cli/commands/quota/history.py index b11b448e..b465b539 100644 --- a/cloudsmith_cli/cli/commands/quota/history.py +++ b/cloudsmith_cli/cli/commands/quota/history.py @@ -104,7 +104,7 @@ def usage(ctx, opts, owner, oss): """ # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) click.echo("Getting quota ... ", nl=False, err=use_stderr) owner = owner[0] diff --git a/cloudsmith_cli/cli/commands/quota/quota.py b/cloudsmith_cli/cli/commands/quota/quota.py index ed9cbc81..94ddb215 100644 --- a/cloudsmith_cli/cli/commands/quota/quota.py +++ b/cloudsmith_cli/cli/commands/quota/quota.py @@ -89,7 +89,7 @@ def usage(ctx, opts, owner, oss): """ # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) click.echo("Getting quota ... ", nl=False, err=use_stderr) owner = owner[0] diff --git a/cloudsmith_cli/cli/commands/repos.py b/cloudsmith_cli/cli/commands/repos.py index 216faed0..428afdea 100644 --- a/cloudsmith_cli/cli/commands/repos.py +++ b/cloudsmith_cli/cli/commands/repos.py @@ -104,7 +104,7 @@ def get(ctx, opts, owner_repo, page, page_size, page_all): (if any). If you're unauthenticated, no results will be returned. """ # Use stderr for messages if the output is something else (e.g. JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) if isinstance(owner_repo, list): if len(owner_repo) == 1: @@ -181,7 +181,7 @@ def create(ctx, opts, owner, repo_config_file): $ cloudsmith repos create your-org repo-config-file.json """ # Use stderr for messages if the output is something else (e.g. JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) repo_config = json.load(repo_config_file) repo_name = repo_config.get("name", None) @@ -312,7 +312,9 @@ def delete(ctx, opts, owner_repo, yes): } prompt = "delete the %(repository)s from the %(namespace)s namespace" % delete_args - if not utils.confirm_operation(prompt, assume_yes=yes): + # Use stderr for messages if the output is something else (e.g. JSON) + use_stderr = utils.should_use_stderr(opts) + if not utils.confirm_operation(prompt, assume_yes=yes, err=use_stderr): return click.secho( diff --git a/cloudsmith_cli/cli/commands/resync.py b/cloudsmith_cli/cli/commands/resync.py index bd8e938c..b60e7cef 100644 --- a/cloudsmith_cli/cli/commands/resync.py +++ b/cloudsmith_cli/cli/commands/resync.py @@ -3,7 +3,7 @@ import click from ...core.api.packages import resync_package as api_resync_package -from .. import decorators, validators +from .. import decorators, utils, validators from ..exceptions import handle_api_exceptions from ..utils import maybe_spinner from .main import main @@ -53,6 +53,7 @@ def resync( ) if no_wait_for_sync: + utils.maybe_print_status_json(opts, {"slug": slug, "status": "OK"}) return wait_for_package_sync( @@ -66,13 +67,17 @@ def resync( attempts=sync_attempts, ) + utils.maybe_print_status_json(opts, {"slug": slug, "status": "OK"}) + def resync_package(ctx, opts, owner, repo, slug, skip_errors): """Resynchronise a package.""" + use_stderr = utils.should_use_stderr(opts) click.echo( "Resynchonising the %(slug)s package ... " % {"slug": click.style(slug, bold=True)}, nl=False, + err=use_stderr, ) context_msg = "Failed to resynchronise package!" @@ -82,4 +87,4 @@ def resync_package(ctx, opts, owner, repo, slug, skip_errors): with maybe_spinner(opts): api_resync_package(owner=owner, repo=repo, identifier=slug) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) diff --git a/cloudsmith_cli/cli/commands/status.py b/cloudsmith_cli/cli/commands/status.py index 53aa7b03..b86a9e20 100644 --- a/cloudsmith_cli/cli/commands/status.py +++ b/cloudsmith_cli/cli/commands/status.py @@ -3,7 +3,7 @@ import click from ...core.api.packages import get_package_status -from .. import decorators, validators +from .. import decorators, utils, validators from ..exceptions import handle_api_exceptions from ..utils import maybe_spinner from .main import main @@ -32,6 +32,8 @@ def status(ctx, opts, owner_repo_package): """ owner, repo, slug = owner_repo_package + use_stderr = utils.should_use_stderr(opts) + click.echo( "Getting status of %(package)s in %(owner)s/%(repo)s ... " % { @@ -40,6 +42,7 @@ def status(ctx, opts, owner_repo_package): "package": click.style(slug, bold=True), }, nl=False, + err=use_stderr, ) context_msg = "Failed to get status of package!" @@ -48,7 +51,7 @@ def status(ctx, opts, owner_repo_package): res = get_package_status(owner, repo, slug) ok, failed, _, status_str, stage_str, reason = res - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) if not stage_str: package_status = status_str @@ -62,13 +65,28 @@ def status(ctx, opts, owner_repo_package): else: status_colour = "magenta" + if utils.maybe_print_as_json( + opts, + { + "ok": ok, + "failed": failed, + "status": status_str, + "stage": stage_str, + "reason": reason, + "slug": slug, + }, + ): + return + click.secho( "The package status is: %(status)s" - % {"status": click.style(package_status, fg=status_colour)} + % {"status": click.style(package_status, fg=status_colour)}, + err=use_stderr, ) if reason: click.secho( f"Reason given: {click.style(reason, fg='yellow')}", fg=status_colour, + err=use_stderr, ) diff --git a/cloudsmith_cli/cli/commands/tags.py b/cloudsmith_cli/cli/commands/tags.py index 66df1fd7..07275195 100644 --- a/cloudsmith_cli/cli/commands/tags.py +++ b/cloudsmith_cli/cli/commands/tags.py @@ -95,10 +95,13 @@ def list_tags(ctx, opts, owner_repo_package): """ owner, repo, package = owner_repo_package + use_stderr = utils.should_use_stderr(opts) + click.echo( "Listing tags for the '%(package)s' package ... " % {"package": click.style(package, bold=True)}, nl=False, + err=use_stderr, ) context_msg = "Failed to list tags for the package!" @@ -108,7 +111,7 @@ def list_tags(ctx, opts, owner_repo_package): owner=owner, repo=repo, identifier=package ) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) _print_tags(opts, package_tags, package_tags_immutable) @@ -158,6 +161,8 @@ def add_tags(ctx, opts, owner_repo_package, tags, immutable): owner, repo, package = owner_repo_package tags = _parse_tags(tags) + use_stderr = utils.should_use_stderr(opts) + click.echo( "Adding '%(tags)s' tag%(s)s to the '%(package)s' package ... " % { @@ -166,6 +171,7 @@ def add_tags(ctx, opts, owner_repo_package, tags, immutable): "s": "s" if len(tags) != 1 else "", }, nl=False, + err=use_stderr, ) context_msg = "Failed to add tags to package!" @@ -178,7 +184,7 @@ def add_tags(ctx, opts, owner_repo_package, tags, immutable): data={"action": "add", "tags": tags, "is_immutable": immutable}, ) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) _print_tags(opts, package_tags, package_tags_immutable) @@ -212,10 +218,13 @@ def clear_tags(ctx, opts, owner_repo_package): """ owner, repo, package = owner_repo_package + use_stderr = utils.should_use_stderr(opts) + click.echo( "Clearing tags on the '%(package)s' package ... " % {"package": click.style(package, bold=True)}, nl=False, + err=use_stderr, ) context_msg = "Failed to clear tags on package!" @@ -225,7 +234,7 @@ def clear_tags(ctx, opts, owner_repo_package): owner=owner, repo=repo, identifier=package, data={"action": "clear"} ) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) _print_tags(opts, package_tags, package_tags_immutable) @@ -265,6 +274,8 @@ def remove_tags(ctx, opts, owner_repo_package, tags): owner, repo, package = owner_repo_package tags = _parse_tags(tags) + use_stderr = utils.should_use_stderr(opts) + click.echo( "Removing '%(tags)s' tag%(s)s from the '%(package)s' package ... " % { @@ -273,6 +284,7 @@ def remove_tags(ctx, opts, owner_repo_package, tags): "s": "s" if len(tags) != 1 else "", }, nl=False, + err=use_stderr, ) context_msg = "Failed to remove tags from package!" @@ -285,7 +297,7 @@ def remove_tags(ctx, opts, owner_repo_package, tags): data={"action": "remove", "tags": tags}, ) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) _print_tags(opts, package_tags, package_tags_immutable) @@ -335,6 +347,8 @@ def replace_tags(ctx, opts, owner_repo_package, tags, immutable): owner, repo, package = owner_repo_package tags = _parse_tags(tags) + use_stderr = utils.should_use_stderr(opts) + click.echo( "Replacing existing with '%(tags)s' tag%(s)s on the '%(package)s' package ... " % { @@ -343,6 +357,7 @@ def replace_tags(ctx, opts, owner_repo_package, tags, immutable): "s": "s" if len(tags) != 1 else "", }, nl=False, + err=use_stderr, ) context_msg = "Failed to replace tags on package!" @@ -355,6 +370,6 @@ def replace_tags(ctx, opts, owner_repo_package, tags, immutable): data={"action": "replace", "tags": tags, "is_immutable": immutable}, ) - click.secho("OK", fg="green") + click.secho("OK", fg="green", err=use_stderr) _print_tags(opts, package_tags, package_tags_immutable) diff --git a/cloudsmith_cli/cli/commands/tokens.py b/cloudsmith_cli/cli/commands/tokens.py index 9b26250d..ad8949f5 100644 --- a/cloudsmith_cli/cli/commands/tokens.py +++ b/cloudsmith_cli/cli/commands/tokens.py @@ -2,9 +2,8 @@ from ...core.api import exceptions, user as api from ...core.config import create_config_files, new_config_messaging -from .. import command, decorators +from .. import command, decorators, utils from ..exceptions import handle_api_exceptions -from ..utils import maybe_print_as_json, maybe_spinner from .main import main @@ -23,6 +22,7 @@ def handle_duplicate_token_error(exc, ctx, opts, save_config, force, json): if not click.confirm( "User already has a token. Would you like to recreate it?", abort=False, + err=json, ): return None return refresh_existing_token_interactive( @@ -47,17 +47,17 @@ def tokens(ctx, opts): @click.pass_context def list_tokens(ctx, opts): """List all user API tokens.""" - use_stderr = opts.output in ("json", "pretty_json") + use_stderr = utils.should_use_stderr(opts) click.echo("Retrieving API tokens... ", nl=False, err=use_stderr) context_msg = "Failed to retrieve API tokens!" with handle_api_exceptions(ctx, opts=opts, context_msg=context_msg): - with maybe_spinner(opts): + with utils.maybe_spinner(opts): tokens = api.list_user_tokens() click.secho("OK", fg="green", err=use_stderr) - if maybe_print_as_json(opts, tokens): + if utils.maybe_print_as_json(opts, tokens): return print_tokens(tokens) @@ -123,7 +123,7 @@ def refresh(ctx, opts, token_slug, force, save_config): ) if new_token: - if maybe_print_as_json(opts, new_token): + if utils.maybe_print_as_json(opts, new_token): return new_token @@ -140,6 +140,8 @@ def refresh_existing_token_interactive( ctx, opts, token_slug=None, save_config=False, force=False, json=False ): """Refresh an existing API token with interactive token selection, or create new if none exist.""" + json = json or utils.should_use_stderr(opts) + use_stderr = json context_msg = "Failed to refresh the token!" if not token_slug: @@ -151,13 +153,17 @@ def refresh_existing_token_interactive( if opts.debug: click.echo(f"Debug: Failed to list tokens with error: {exc}", err=True) click.echo( - "Unable to list existing tokens. Creating a new token instead..." + "Unable to list existing tokens. Creating a new token instead...", + err=use_stderr, ) - return _create(ctx, opts, save_config=save_config, force=force) + return _create(ctx, opts, save_config=save_config, force=force, json=json) if not api_tokens: - click.echo("No existing tokens found. Creating a new token instead...") - return _create(ctx, opts, save_config=save_config, force=force) + click.echo( + "No existing tokens found. Creating a new token instead...", + err=use_stderr, + ) + return _create(ctx, opts, save_config=save_config, force=force, json=json) if not json: click.echo("Current tokens:") @@ -165,7 +171,8 @@ def refresh_existing_token_interactive( if not force: token_slug = click.prompt( - "Please enter the slug_perm of the token you would like to refresh" + "Please enter the slug_perm of the token you would like to refresh", + err=use_stderr, ) else: # Use the first available slug_perm when force is enabled @@ -175,9 +182,13 @@ def refresh_existing_token_interactive( if not json: click.echo(f"Refreshing token {token_slug}... ", nl=False) + else: + # In JSON mode, print info to stderr + click.echo(f"Refreshing token {token_slug}... ", nl=False, err=True) + try: with handle_api_exceptions(ctx, opts=opts, context_msg=context_msg): - with maybe_spinner(opts): + with utils.maybe_spinner(opts): new_token = api.refresh_user_token(token_slug) if save_config: @@ -186,7 +197,7 @@ def refresh_existing_token_interactive( ) new_config_messaging(has_errors, opts, create, api_key=new_token.key) - if maybe_print_as_json(opts, new_token): + if utils.maybe_print_as_json(opts, new_token): return if not json: @@ -198,35 +209,36 @@ def refresh_existing_token_interactive( # If refresh fails due to API error, offer to create a new token instead if opts.debug: click.echo(f"\nDebug: Refresh failed with error: {exc}", err=True) - click.echo("\nRefresh failed. Creating a new token instead...") - return _create(ctx, opts, save_config=save_config, force=force) + click.echo("\nRefresh failed. Creating a new token instead...", err=use_stderr) + return _create(ctx, opts, save_config=save_config, force=force, json=json) def _create(ctx, opts, save_config=False, force=False, json=False): """Create a new API token.""" + json = json or utils.should_use_stderr(opts) + try: new_token = api.create_user_token_saml() if new_token: - if json: + # NOTE: This mutation is necessary during the deprecation period of the --json flag. + if json and opts.output not in ("json", "pretty_json"): opts.output = "json" - if maybe_print_as_json(opts, new_token): - return - if maybe_print_as_json(opts, new_token): + + if utils.maybe_print_as_json(opts, new_token): return - if not json: - click.echo( - f"New token value: {click.style(new_token.key, fg='magenta')}" - ) + + click.echo(f"New token value: {click.style(new_token.key, fg='magenta')}") return new_token return new_token except exceptions.ApiException as exc: if exc.status == 401: - click.echo(f"{exc.detail}") + click.echo(f"{exc.detail}", err=True) return - if json: + # NOTE: This mutation is necessary during the deprecation period of the --json flag. + if json and opts.output not in ("json", "pretty_json"): opts.output = "json" if exc.status == 400: new_token = handle_duplicate_token_error( diff --git a/cloudsmith_cli/cli/commands/upstream.py b/cloudsmith_cli/cli/commands/upstream.py index 3500f5ad..290b6df5 100644 --- a/cloudsmith_cli/cli/commands/upstream.py +++ b/cloudsmith_cli/cli/commands/upstream.py @@ -173,8 +173,7 @@ def build_upstream_list_command(upstream_fmt): def func(ctx, opts, owner_repo, page, page_size, page_all): owner, repo = owner_repo - # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + use_stderr = utils.should_use_stderr(opts) if not use_stderr: click.echo("Getting upstreams... ", nl=False, err=use_stderr) @@ -418,7 +417,7 @@ def func(ctx, opts, owner_repo_slug_perm, yes): "delete the %(slug_perm)s upstream from the %(owner)s/%(repo)s repository" % delete_args ) - if not utils.confirm_operation(prompt, assume_yes=yes): + if not utils.confirm_operation(prompt, assume_yes=yes, err=use_stderr): return if not use_stderr: diff --git a/cloudsmith_cli/cli/commands/whoami.py b/cloudsmith_cli/cli/commands/whoami.py index 562304e1..cf53c1c0 100644 --- a/cloudsmith_cli/cli/commands/whoami.py +++ b/cloudsmith_cli/cli/commands/whoami.py @@ -3,9 +3,8 @@ import click from ...core.api.user import get_user_brief -from .. import decorators +from .. import decorators, utils from ..exceptions import handle_api_exceptions -from ..utils import maybe_print_as_json, maybe_spinner from .main import main @@ -17,7 +16,7 @@ @click.pass_context def whoami(ctx, opts): """Retrieve your current authentication status.""" - use_stderr = opts.output in ("json", "pretty_json") + use_stderr = utils.should_use_stderr(opts) click.echo( "Retrieving your authentication status from the API ... ", @@ -27,7 +26,7 @@ def whoami(ctx, opts): context_msg = "Failed to retrieve your authentication status!" with handle_api_exceptions(ctx, opts=opts, context_msg=context_msg): - with maybe_spinner(opts): + with utils.maybe_spinner(opts): is_auth, username, email, name = get_user_brief() click.secho("OK", fg="green", err=use_stderr) @@ -38,7 +37,7 @@ def whoami(ctx, opts): "name": name, } - if maybe_print_as_json(opts, data): + if utils.maybe_print_as_json(opts, data): return click.echo("You are authenticated as:") diff --git a/cloudsmith_cli/cli/exceptions.py b/cloudsmith_cli/cli/exceptions.py index e5aedc7a..162b8b8f 100644 --- a/cloudsmith_cli/cli/exceptions.py +++ b/cloudsmith_cli/cli/exceptions.py @@ -16,67 +16,103 @@ def handle_api_exceptions( ): """Context manager that handles API exceptions.""" # flake8: ignore=C901 + # Use stderr for messages if the output is something else (e.g. # JSON) - use_stderr = opts.output != "pretty" + is_json_output = getattr(opts, "output", None) in ("json", "pretty_json") + use_stderr = is_json_output try: yield except ApiException as exc: - if nl: - click.echo(err=use_stderr) - click.secho("ERROR: ", fg="red", nl=False, err=use_stderr) - else: - click.secho("ERROR", fg="red", err=use_stderr) - context_msg = context_msg or "Failed to perform operation!" - click.secho( - "%(context)s (status: %(code)s - %(code_text)s)" - % { - "context": context_msg, - "code": exc.status, - "code_text": exc.status_description, - }, - fg="red", - err=use_stderr, - ) - detail, fields = get_details(exc) - if detail or fields: - click.echo(err=use_stderr) - - if detail: - click.secho( - "Detail: %(detail)s" - % {"detail": click.style(detail, fg="red", bold=False)}, - bold=True, - err=use_stderr, - ) + hint = get_error_hint(ctx, opts, exc) + + if is_json_output: + # Construct JSON error object + error_data = { + "detail": detail or exc.status_description, + "help": { + "context": context_msg, + "hint": hint, + }, + "meta": { + "code": exc.status, + "description": exc.status_description, + }, + } if fields: - for k, v in fields.items(): - field = "%s Field" % k.capitalize() - click.secho( - "%(field)s: %(message)s" - % { - "field": click.style(field, bold=True), - "message": click.style(v, fg="red"), - }, - err=use_stderr, - ) + error_data["fields"] = fields + + # Print to stdout + import json - hint = get_error_hint(ctx, opts, exc) - if hint: click.echo( - f"Hint: {click.style(hint, fg='yellow')}", + json.dumps( + error_data, indent=4 if opts.output == "pretty_json" else None + ) + ) + + else: + # Standard CLI output to stderr (or interleaved if output != pretty, but we force use_stderr now) + if nl: + click.echo(err=use_stderr) + click.secho("ERROR: ", fg="red", nl=False, err=use_stderr) + else: + click.secho("ERROR", fg="red", err=use_stderr) + + click.secho( + "%(context)s (status: %(code)s - %(code_text)s)" + % { + "context": context_msg, + "code": exc.status, + "code_text": exc.status_description, + }, + fg="red", err=use_stderr, ) - if opts.verbose and not opts.debug: - if exc.headers: + if detail or fields: click.echo(err=use_stderr) - click.echo("Headers in Reply:", err=use_stderr) - for k, v in exc.headers.items(): - click.echo(f"{k} = {v}", err=use_stderr) + + if detail: + click.secho( + "Detail: %(detail)s" + % {"detail": click.style(detail, fg="red", bold=False)}, + bold=True, + err=use_stderr, + ) + + if fields: + for k, v in fields.items(): + field = "%s Field" % k.capitalize() + + # Flatten list/tuple error messages for text output + if isinstance(v, (list, tuple)): + v = " ".join(v) + + click.secho( + "%(field)s: %(message)s" + % { + "field": click.style(field, bold=True), + "message": click.style(v, fg="red"), + }, + err=use_stderr, + ) + + if hint: + click.echo( + f"Hint: {click.style(hint, fg='yellow')}", + err=use_stderr, + ) + + if opts.verbose and not opts.debug: + if exc.headers: + click.echo(err=use_stderr) + click.echo("Headers in Reply:", err=use_stderr) + for k, v in exc.headers.items(): + click.echo(f"{k} = {v}", err=use_stderr) if reraise_on_error: raise @@ -100,10 +136,11 @@ def get_details(exc): except (TypeError, KeyError): field_detail = v - if isinstance(field_detail, (list, tuple)): - field_detail = " ".join(field_detail) - if k == "non_field_errors": + # Ensure we handle list/tuple for non_field_errors details joining + if isinstance(field_detail, (list, tuple)): + field_detail = " ".join(field_detail) + if detail: detail += " " + field_detail else: diff --git a/cloudsmith_cli/cli/utils.py b/cloudsmith_cli/cli/utils.py index dd963c60..4392127e 100644 --- a/cloudsmith_cli/cli/utils.py +++ b/cloudsmith_cli/cli/utils.py @@ -191,10 +191,25 @@ def confirm_operation(prompt, prefix=None, assume_yes=False, err=False): @contextmanager def maybe_spinner(opts): - """Only activate the spinner if not in debug mode.""" - if opts.debug: + """Only activate the spinner if not in debug mode or using json output.""" + if opts.debug or get_output_format(opts) in ("json", "pretty_json"): # No spinner yield else: with spinner() as spin: yield spin + + +def get_output_format(opts): + """Get the output format from opts.""" + return getattr(opts, "output", None) + + +def should_use_stderr(opts): + """Check if stdout should be avoided for informational messages.""" + return get_output_format(opts) in ("json", "pretty_json") + + +def maybe_print_status_json(opts, status_dict): + """Maybe print a status dict as JSON.""" + return maybe_print_as_json(opts, status_dict) diff --git a/cloudsmith_cli/cli/webserver.py b/cloudsmith_cli/cli/webserver.py index 381b3433..bd88c121 100644 --- a/cloudsmith_cli/cli/webserver.py +++ b/cloudsmith_cli/cli/webserver.py @@ -214,7 +214,7 @@ def do_GET(self): if two_factor_token: totp_token = click.prompt( - "Please enter your 2FA token", hide_input=True, type=str + "Please enter your 2FA token", hide_input=True, type=str, err=True ) access_token, refresh_token = exchange_2fa_token(