diff --git a/packages/reflex-base/news/+reserve-stdout.feature.md b/packages/reflex-base/news/+reserve-stdout.feature.md new file mode 100644 index 00000000000..4a2f3b82412 --- /dev/null +++ b/packages/reflex-base/news/+reserve-stdout.feature.md @@ -0,0 +1 @@ +`reflex_base.utils.log.reserve_stdout()` reserves stdout for a machine-readable document, rendering log records, tables, rules, prompts, spinners and progress bars to stderr for as long as it is set. diff --git a/packages/reflex-base/src/reflex_base/utils/console.py b/packages/reflex-base/src/reflex_base/utils/console.py index 105e59a8a13..11541720ff6 100644 --- a/packages/reflex-base/src/reflex_base/utils/console.py +++ b/packages/reflex-base/src/reflex_base/utils/console.py @@ -34,6 +34,17 @@ _console = Console(highlight=False) _console_stderr = Console(stderr=True, highlight=False) + +def _human_console() -> Console: + """Get the console human-readable output renders to. + + Returns: + The stderr console while stdout is reserved for a machine-readable + document, the stdout one otherwise. + """ + return _console_stderr if _log.is_stdout_reserved() else _console + + # Deprecated features who's warning has been printed. _EMITTED_DEPRECATION_WARNINGS = set() @@ -108,7 +119,7 @@ def print(msg: str, *, dedupe: bool = False, level: str = "info", **kwargs): if msg in _EMITTED_PRINTS: return _EMITTED_PRINTS.add(msg) - _console.print(msg, **kwargs) + _human_console().print(msg, **kwargs) def _print_stderr(msg: str, *, dedupe: bool = False, level: str = "error", **kwargs): @@ -250,7 +261,7 @@ def log(msg: str, *, dedupe: bool = False, **kwargs): if _log.is_json_mode(): _log.emit_json_print(msg) else: - _console.log(msg, **kwargs) + _human_console().log(msg, **kwargs) if should_use_log_file_console(): print_to_log_file(msg, **kwargs) @@ -264,7 +275,7 @@ def rule(title: str, **kwargs): """ if _log.is_json_mode(): return - _console.rule(title, **kwargs) + _human_console().rule(title, **kwargs) def warn(msg: str, *, dedupe: bool = False, **kwargs): @@ -459,8 +470,15 @@ def ask( Returns: A string with the user input. """ + # A prompt is human output like any other, and the one that must not land + # on a reserved stdout: it blocks, so a caller parsing the document reads + # the question as data and never answers it. return Prompt.ask( - question, choices=choices, default=default, show_choices=show_choices + question, + choices=choices, + default=default, + show_choices=show_choices, + console=_human_console(), ) @@ -494,7 +512,7 @@ def print_table( for row in tabular_data: table.add_row(*row) - _console.print(table) + _human_console().print(table) def progress(): @@ -507,7 +525,10 @@ def progress(): *Progress.get_default_columns()[:-1], MofNCompleteColumn(), TimeElapsedColumn(), - disable=_log.is_json_mode(), + # A bar is decoration, and it redraws in place: there is nowhere to + # put it in a machine-readable stream, and nothing to draw it over + # once stdout belongs to a document. + disable=_log.is_json_mode() or _log.is_stdout_reserved(), ) @@ -523,7 +544,7 @@ def status(*args, **kwargs): """ if _log.is_json_mode(): return _log._quiet_console.status(*args, **kwargs) - return _console.status(*args, **kwargs) + return _human_console().status(*args, **kwargs) @contextlib.contextmanager diff --git a/packages/reflex-base/src/reflex_base/utils/log.py b/packages/reflex-base/src/reflex_base/utils/log.py index ce74a1eb3d3..06d12fda2e1 100644 --- a/packages/reflex-base/src/reflex_base/utils/log.py +++ b/packages/reflex-base/src/reflex_base/utils/log.py @@ -78,6 +78,9 @@ # Console that renders nowhere, backing interactive rich features in JSON mode. _quiet_console = Console(quiet=True) +# Whether stdout carries a machine-readable document rather than human output. +_stdout_reserved = False + # The current log level. _log_level = LogLevel.INFO @@ -198,7 +201,9 @@ def emit(self, record: logging.LogRecord): """ try: style, prefix = _style_for(record) - console = _console_stderr if record.levelno >= logging.ERROR else _console + console = ( + _console_stderr if record.levelno >= logging.ERROR else human_console() + ) # Records may carry a rich Progress to print through, so the # message lands above an active progress bar. progress = getattr(record, "progress", None) @@ -242,7 +247,7 @@ def _write_json(payload: dict, *, stderr: bool): payload: The record fields. stderr: Whether the record targets stderr. """ - stream = sys.stderr if stderr else sys.stdout + stream = sys.stderr if stderr or _stdout_reserved else sys.stdout stream.write(json.dumps(payload, default=str) + "\n") stream.flush() @@ -472,6 +477,38 @@ def is_json_mode() -> bool: return environment.REFLEX_LOG_JSON.get() +def reserve_stdout(reserved: bool = True): + """Reserve stdout for a machine-readable document. + + A command that writes structured output (``--json``) owns stdout for the + duration, so every human-readable message -- log records, tables, spinners + -- renders to stderr instead and cannot land in the middle of the document. + + Args: + reserved: Whether stdout carries data rather than human output. + """ + global _stdout_reserved + _stdout_reserved = reserved + + +def is_stdout_reserved() -> bool: + """Check whether stdout is reserved for a machine-readable document. + + Returns: + True if human-readable output has to go to stderr. + """ + return _stdout_reserved + + +def human_console() -> Console: + """Get the console human-readable output renders to. + + Returns: + The stderr console while stdout is reserved, the stdout one otherwise. + """ + return _console_stderr if _stdout_reserved else _console + + def set_json_mode(enabled: bool): """Enable or disable machine-readable JSON log output. @@ -633,7 +670,8 @@ def ensure_configured(): def _reset(): """Detach the sinks and restore propagation (test teardown helper).""" - global _configured + global _configured, _stdout_reserved + _stdout_reserved = False for handler in (_console_handler(), _json_handler(), _active_file_handler): if handler is not None: _REFLEX_LOGGER.removeHandler(handler) diff --git a/packages/reflex-hosting-cli/news/+agent-friendly-cli.feature.md b/packages/reflex-hosting-cli/news/+agent-friendly-cli.feature.md new file mode 100644 index 00000000000..986ac1e2467 --- /dev/null +++ b/packages/reflex-hosting-cli/news/+agent-friendly-cli.feature.md @@ -0,0 +1 @@ +Every `reflex cloud` command now takes `--json`, writing one JSON document to stdout while human-readable messages move to stderr, so the output is parseable without reading a Rich table. `--interactive` defaults to whether stdout is a terminal, so a pipe, a CI job or an agent is never left waiting at a prompt instead of exiting. `reflex cloud apps logs --follow` now defaults to off: following prompts between pages and never returns on its own, so it is opt-in. `reflex deploy` takes the same terminal-derived `--interactive`, so a deploy off a TTY reports a missing token instead of waiting at a prompt; it keeps `--json` for log records only, since its progress is a stream rather than a result. diff --git a/packages/reflex-hosting-cli/src/reflex_cli/utils/console.py b/packages/reflex-hosting-cli/src/reflex_cli/utils/console.py index 81961c34d7e..d12ab94574e 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/utils/console.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/utils/console.py @@ -11,7 +11,7 @@ from typing import overload from reflex_cli.constants.base import LogLevel -from reflex_cli.utils.log import HAS_REFLEX_BASE, is_json_mode +from reflex_cli.utils.log import HAS_REFLEX_BASE, is_json_mode, is_stdout_reserved from reflex_cli.utils.log import set_log_level as _set_log_level if HAS_REFLEX_BASE: @@ -28,6 +28,16 @@ from rich.table import Table _console = Console(highlight=False) + _console_stderr = Console(stderr=True, highlight=False) + + def _human_console() -> Console: + """Resolve the console human-readable output belongs on. + + Returns: + The stderr console while stdout is carrying a machine-readable + document, and the stdout console otherwise. + """ + return _console_stderr if is_stdout_reserved() else _console def print(msg: str, **kwargs): """Print a message. @@ -36,7 +46,7 @@ def print(msg: str, **kwargs): msg: The message to print. kwargs: Keyword arguments to pass to the print function. """ - _console.print(msg, **kwargs) + _human_console().print(msg, **kwargs) def print_table( tabular_data: list[list[str]], @@ -60,7 +70,7 @@ def print_table( for row in tabular_data: table.add_row(*row) - _console.print(table) + _human_console().print(table) def rule(title: str, **kwargs): """Print a horizontal rule with a title. @@ -69,7 +79,7 @@ def rule(title: str, **kwargs): title: The title of the rule. kwargs: Keyword arguments to pass to the print function. """ - _console.rule(title, **kwargs) + _human_console().rule(title, **kwargs) @overload def ask( @@ -105,7 +115,11 @@ def ask( A string with the user input. """ return Prompt.ask( - question, choices=choices, default=default, show_choices=show_choices + question, + choices=choices, + default=default, + show_choices=show_choices, + console=_human_console(), ) def progress(): @@ -130,7 +144,7 @@ def status(*args, **kwargs): Returns: A new status. """ - return _console.status(*args, **kwargs) + return _human_console().status(*args, **kwargs) def set_log_level(log_level: LogLevel | str | None): diff --git a/packages/reflex-hosting-cli/src/reflex_cli/utils/hosting.py b/packages/reflex-hosting-cli/src/reflex_cli/utils/hosting.py index dc63d538d0f..55f7e70b8e2 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/utils/hosting.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/utils/hosting.py @@ -3285,13 +3285,16 @@ def read_config( return Config.from_yaml_or_toml_or_none() -def generate_config(interactive: bool = True, token: str | None = None): +def generate_config(interactive: bool = True, token: str | None = None) -> Path | None: """Generate the config file with app-based prefilling. Args: interactive: Whether to use interactive mode for authentication and app selection. token: An existing authentication token to use instead of interactive auth. + Returns: + The path of the config file written, or None if none was. + Raises: click.exceptions.Exit: If authentication fails or user cancels operation. """ @@ -3299,11 +3302,12 @@ def generate_config(interactive: bool = True, token: str | None = None): import yaml except ImportError: logger.error("Please install PyYAML to use this command: pip install pyyaml") - return + return None - if Path("cloud.yml").exists(): + config_path = Path("cloud.yml") + if config_path.exists(): logger.error("cloud.yml already exists.") - return + return None try: authenticated_client = get_authenticated_client( @@ -3344,13 +3348,13 @@ def generate_config(interactive: bool = True, token: str | None = None): ) default = {"name": current_dir_name} - with Path("cloud.yml").open("w") as config_file: + with config_path.open("w") as config_file: yaml.dump(default, config_file, default_flow_style=False, sort_keys=False) logger.log(log.SUCCESS, "cloud.yml created successfully.") logger.info( "For more configuration options, see: https://reflex.dev/docs/hosting/config-file/" ) - return + return config_path def log_out_on_browser(): diff --git a/packages/reflex-hosting-cli/src/reflex_cli/utils/log.py b/packages/reflex-hosting-cli/src/reflex_cli/utils/log.py index 1cb1b9466cd..f6069992515 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/utils/log.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/utils/log.py @@ -82,8 +82,12 @@ def emit(self, record: logging.LogRecord): """ try: style, prefix = _style_for_level(record.levelno) + # Errors always go to stderr; everything else joins them there + # while stdout is carrying a document. console = ( - _console_stderr if record.levelno >= logging.ERROR else _console + _console_stderr + if record.levelno >= logging.ERROR or is_stdout_reserved() + else _console ) # Markup is opt-in per record (``extra={"rich": True}``); plain # messages keep their literal brackets. @@ -129,3 +133,38 @@ def set_log_level(log_level: LogLevel | None): # no-op when the handler is already attached, so this stays idempotent. _CLI_LOGGER.propagate = False _CLI_LOGGER.addHandler(_handler) + + +# Asked separately from the names above, and that separation is the whole point. +# The reservation is younger than the rest of this shim: every published +# reflex-base exports SUCCESS, is_json_mode and set_log_level, and none of them +# exports these two. Importing all five together sent an installation with a +# perfectly good reflex-base down the fallback path entirely -- losing its +# console, its log parenting and is_json_mode to acquire a feature it was only +# ever meant to go without. +try: + from reflex_base.utils.log import is_stdout_reserved as is_stdout_reserved + from reflex_base.utils.log import reserve_stdout as reserve_stdout + +except ImportError: + # Tracked here rather than delegated, so `--json` keeps its stdout even + # against a reflex-base that has never heard of the reservation. Only this + # shim's own sinks consult it; a reflex-base console cannot be told. + _stdout_reserved = False + + def reserve_stdout(reserved: bool = True) -> None: + """Reserve stdout for a machine-readable document. + + Args: + reserved: Whether stdout carries data rather than human output. + """ + global _stdout_reserved + _stdout_reserved = reserved + + def is_stdout_reserved() -> bool: + """Check whether stdout is reserved for machine-readable output. + + Returns: + True while a document owns stdout. + """ + return _stdout_reserved diff --git a/packages/reflex-hosting-cli/src/reflex_cli/utils/output.py b/packages/reflex-hosting-cli/src/reflex_cli/utils/output.py new file mode 100644 index 00000000000..209a120c095 --- /dev/null +++ b/packages/reflex-hosting-cli/src/reflex_cli/utils/output.py @@ -0,0 +1,202 @@ +"""Machine-readable output, and the shared options that turn it on. + +An agent driving the cloud CLI needs two things a person at a terminal does +not: output it can parse without regexing a Rich table, and the certainty that +nothing will stop and wait for a keystroke. ``--json`` answers the first and +reserves stdout for the document while it does; ``--interactive`` answers the +second by defaulting to whether stdout is a terminal. +""" + +from __future__ import annotations + +import json +import sys +from collections.abc import Sequence +from typing import Any + +import click + +# Through the CLI's own shim, not reflex_base directly: the hosting CLI has to +# import against a reflex-base that predates these functions, which is what +# tests/units/reflex_cli/utils/test_log.py pins. +from reflex_cli.utils import log + +# The spellings that ask for JSON on the command line, and the one that +# refuses it. Read straight off argv so the group callback can reserve stdout +# before click has parsed the subcommand's options -- anything it says would +# otherwise land on stdout ahead of the document. +_JSON_FLAGS = frozenset({"--json", "-j"}) +_JSON_SHORT = "j" +_NO_JSON_FLAG = "--no-json" + + +def _json_flag_state(arg: str) -> bool | None: + """Read what one command-line argument says about JSON output. + + Args: + arg: A single command-line argument. + + Returns: + True if it asks for JSON, False if it refuses it, None if it says + nothing either way. + """ + if arg == _NO_JSON_FLAG: + return False + if arg in _JSON_FLAGS: + return True + # Short flags combine, so `-ij` is `-i -j`. Reading them means this scan + # can also fire on a `-j` that click would take as some other option's + # value, which is the direction to be wrong in: a message on stderr costs + # a little context, one inside the document costs the whole parse. + return ( + True + if len(arg) > 1 + and arg.startswith("-") + and not arg.startswith("--") + and _JSON_SHORT in arg[1:] + else None + ) + + +def stdout_is_tty() -> bool: + """Check whether stdout is attached to a terminal. + + Returns: + True if somebody is plausibly watching, False under a pipe, a CI job + or an agent. + """ + isatty = getattr(sys.stdout, "isatty", None) + if isatty is None: + return False + try: + return bool(isatty()) + except ValueError: + # A closed stream. Nobody is answering a prompt on it either way. + return False + + +def _resolve_interactive( + ctx: click.Context, param: click.Parameter, value: bool | None +) -> bool: + """Resolve an unset ``--interactive`` against the terminal. + + Args: + ctx: The click context. + param: The click parameter. + value: The flag's value, or None when neither spelling was passed. + + Returns: + Whether the command may prompt. + """ + return stdout_is_tty() if value is None else value + + +interactive_option = click.option( + "--interactive/--no-interactive", + "-i/", + "interactive", + default=None, + callback=_resolve_interactive, + help="Whether to prompt for confirmations and choices. Defaults to on when " + "stdout is a terminal and off otherwise, so a pipe, a CI job or an agent is " + "never left waiting at a prompt.", +) + + +def json_requested(argv: Sequence[str] | None = None) -> bool: + """Check whether a command line asks for JSON output. + + Scanned back to front, so the last flag decides -- the same answer click + reaches for a boolean flag pair, which a set membership test cannot give: + ``--no-json --json`` enables JSON and ``--json --no-json`` does not. + + Args: + argv: The arguments to inspect; defaults to this process's own. + + Returns: + True if the command line asks for JSON output. + """ + args = sys.argv[1:] if argv is None else argv + for arg in reversed(list(args)): + if (state := _json_flag_state(arg)) is not None: + return state + return False + + +def _hold_reservation(ctx: click.Context, reserved: bool) -> None: + """Reserve stdout for this context, releasing it again when it closes. + + The reservation is process-global, so without an explicit release a + ``--json`` command leaves every later log line in the process writing to + stderr -- which a CLI process never notices, and an embedding one or a + second run in the same interpreter does. + + Args: + ctx: The click context whose lifetime the reservation follows. + reserved: Whether stdout carries data rather than human output. + """ + previous = log.is_stdout_reserved() + log.reserve_stdout(reserved) + ctx.call_on_close(lambda: log.reserve_stdout(previous)) + + +def reserve_stdout_for_argv( + argv: Sequence[str] | None = None, *, ctx: click.Context | None = None +) -> None: + """Reserve stdout up front when the command line asks for JSON. + + Always writes the reservation rather than only setting it, so a long-lived + process (tests, an embedded runner) cannot inherit the previous + invocation's answer. + + Args: + argv: The arguments to inspect; defaults to this process's own. + ctx: The click context to release the reservation with, if there is one. + """ + reserved = json_requested(argv) + if ctx is None: + log.reserve_stdout(reserved) + return + _hold_reservation(ctx, reserved) + + +def _reserve_stdout(ctx: click.Context, param: click.Parameter, value: bool) -> bool: + """Reserve stdout for the document once ``--json`` is parsed. + + Args: + ctx: The click context. + param: The click parameter. + value: Whether JSON output was asked for. + + Returns: + The flag's value, unchanged. + """ + if value: + _hold_reservation(ctx, True) + return value + + +json_option = click.option( + "--json/--no-json", + "-j", + "as_json", + is_flag=True, + is_eager=True, + callback=_reserve_stdout, + help="Output the result as a single JSON document on stdout. Human-readable " + "messages go to stderr instead, so stdout stays parseable.", +) + + +def print_json(payload: Any) -> None: + """Write one JSON document to stdout. + + Deliberately not routed through :mod:`reflex_cli.utils.console`: this is the + output the command was asked for, not a message about it, so it goes to + stdout even while the console renders to stderr, and is never wrapped in a + log record. + + Args: + payload: The value to serialize. + """ + click.echo(json.dumps(payload, default=str)) diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/apps.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/apps.py index 7ca6337d5f3..d889c7dd392 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/apps.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/apps.py @@ -2,7 +2,6 @@ from __future__ import annotations -import json import logging from typing import Any @@ -20,6 +19,7 @@ ScaleParamError, ScaleTypeError, ) +from reflex_cli.utils.output import interactive_option, json_option, print_json logger = logging.getLogger(__name__) @@ -90,20 +90,8 @@ def _resolve_app_id( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def app_history( app_id: str | None, app_name: str | None, @@ -147,7 +135,7 @@ def app_history( history = hosting.get_app_history(app_id=app_id, client=authenticated_client) if as_json: - console.print(json.dumps(history)) + print_json(history) return if history: headers = list(history[0].keys()) @@ -173,19 +161,15 @@ def app_history( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def app_rollback( deployment_id: str, app_id: str | None, app_name: str | None, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Roll an app back to a previous deployment. @@ -215,6 +199,13 @@ def app_rollback( != "y" ): logger.info("Rollback cancelled.") + if as_json: + print_json({ + "app_id": app_id, + "deployment_id": deployment_id, + "rolled_back": False, + "cancelled": True, + }) return result = hosting.rollback_deployment( @@ -223,6 +214,14 @@ def app_rollback( if result: logger.error(result) raise click.exceptions.Exit(1) + if as_json: + print_json({ + "app_id": app_id, + "deployment_id": deployment_id, + "rolled_back": True, + "cancelled": False, + }) + return logger.log(log.SUCCESS, f"Rollback to deployment {deployment_id} started.") console.print( f"Track progress with `reflex cloud apps status {deployment_id} " @@ -249,13 +248,8 @@ def app_rollback( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def app_describe( deployment_id: str, description: str, @@ -263,6 +257,7 @@ def app_describe( app_name: str | None, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Set or clear the changelog note on a past deployment. @@ -288,6 +283,13 @@ def app_describe( if result: logger.error(result) raise click.exceptions.Exit(1) + if as_json: + print_json({ + "app_id": app_id, + "deployment_id": deployment_id, + "description": description, + }) + return if description.strip(): logger.log( log.SUCCESS, f"Updated description for deployment {deployment_id}." @@ -304,16 +306,12 @@ def app_describe( @apps_cli.command("build-logs") @click.argument("deployment_id", required=True) @click.option("--token", help="The authentication token.") -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def deployment_build_logs( deployment_id: str, token: str | None, + as_json: bool, interactive: bool, ): """Retrieve the build logs for a specific deployment.""" @@ -326,6 +324,9 @@ def deployment_build_logs( logs = hosting.get_deployment_build_logs( deployment_id=deployment_id, client=authenticated_client ) + if as_json: + print_json({"deployment_id": deployment_id, "logs": logs}) + return console.print(logs) except NotAuthenticatedError as err: logger.error("You are not authenticated. Run `reflex login` to authenticate.") @@ -344,18 +345,14 @@ def deployment_build_logs( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def deployment_status( deployment_id: str, watch: bool, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Retrieve the status of a specific deployment.""" @@ -368,15 +365,33 @@ def deployment_status( token=token, interactive=interactive ) if watch: - status = hosting.watch_deployment_status( + succeeded = hosting.watch_deployment_status( deployment_id=deployment_id, client=authenticated_client ) - if status is False: + if as_json: + # Re-read once the watch ends: the watch itself reports + # progress through the log stream and returns only whether it + # got there, which is not a status a caller can act on. + print_json({ + "deployment_id": deployment_id, + "status": hosting.get_deployment_status( + deployment_id=deployment_id, client=authenticated_client + ), + "success": succeeded, + }) + if succeeded is False: raise click.exceptions.Exit(1) else: status = hosting.get_deployment_status( deployment_id=deployment_id, client=authenticated_client ) + if as_json: + print_json({ + "deployment_id": deployment_id, + "status": status, + "success": "failed" not in status, + }) + return logger.error(status) if "failed" in status else console.print(status) except NotAuthenticatedError as err: logger.error("You are not authenticated. Run `reflex login` to authenticate.") @@ -393,18 +408,14 @@ def deployment_status( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def stop_app( app_id: str | None, app_name: str | None, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Stop a running application.""" @@ -441,10 +452,12 @@ def stop_app( raise click.exceptions.Exit(1) result = hosting.stop_app(app_id=app_id, client=authenticated_client) + failed = bool(result) and "failed" in result + if as_json: + print_json({"app_id": app_id, "stopped": not failed, "message": result}) + return if result: - logger.error(result) if "failed" in result else logger.log( - log.SUCCESS, result - ) + logger.error(result) if failed else logger.log(log.SUCCESS, result) except NotAuthenticatedError as err: logger.error("You are not authenticated. Run `reflex login` to authenticate.") raise click.exceptions.Exit(1) from err @@ -460,18 +473,14 @@ def stop_app( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def start_app( app_id: str | None, app_name: str | None, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Start a stopped application.""" @@ -507,10 +516,12 @@ def start_app( raise click.exceptions.Exit(1) result = hosting.start_app(app_id=app_id, client=authenticated_client) + failed = bool(result) and "failed" in result + if as_json: + print_json({"app_id": app_id, "started": not failed, "message": result}) + return if result: - logger.error(result) if "failed" in result else logger.log( - log.SUCCESS, result - ) + logger.error(result) if failed else logger.log(log.SUCCESS, result) except NotAuthenticatedError as err: logger.error("You are not authenticated. Run `reflex login` to authenticate.") raise click.exceptions.Exit(1) from err @@ -526,18 +537,14 @@ def start_app( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def delete_app( app_id: str | None, app_name: str | None, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Delete an application.""" @@ -581,6 +588,12 @@ def delete_app( ) except GetAppError: logger.warning(f"No application found with ID '{app_id}'") + if as_json: + print_json({ + "app_id": app_id, + "deleted": False, + "message": f"No application found with ID '{app_id}'", + }) return if not app_result: logger.warning(f"App with ID '{app_id}' not found.") @@ -617,9 +630,26 @@ def delete_app( != "y" ): logger.info("Deletion cancelled.") + if as_json: + print_json({ + "app_id": app_id, + "deleted": False, + "cancelled": True, + }) return result = hosting.delete_app(app_id=app_id, client=authenticated_client) + if as_json: + # A refusal comes back as a message rather than as an exception, so + # the document has to read it too: reporting the call as a deletion + # is how a caller ends up believing an app is gone. + failed = result is None or (isinstance(result, str) and "failed" in result) + print_json({ + "app_id": app_id, + "deleted": not failed, + "message": result, + }) + return if result: logger.warning(result) except NotAuthenticatedError as err: @@ -640,17 +670,17 @@ def delete_app( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option @click.option("--cursor", type=str, help="The cursor for pagination.") @click.option("--pretty", type=bool, help="Use pretty printing for logs.") @click.option( - "--follow", type=bool, default=True, help="Asks to continue to query logs." + "--follow", + type=bool, + default=False, + help="After printing a page, prompt to fetch the next one. Off by default: " + "the prompt never returns on its own, so a script or an agent that asked " + "for logs would hang instead of exiting.", ) def app_logs( app_id: str | None, @@ -660,10 +690,11 @@ def app_logs( start: int | None, end: int | None, loglevel: str, + as_json: bool, interactive: bool, cursor: str | None = None, pretty: bool = False, - follow: bool = True, + follow: bool = False, ): """Retrieve logs for a given application.""" import pprint @@ -706,6 +737,11 @@ def app_logs( logger.error("must provide both start and end") raise click.exceptions.Exit(1) + # Following means prompting between pages, which never returns on its + # own, so it needs somebody at the terminal and a stream that is not + # carrying a JSON document. + following = follow and interactive and not as_json + while True: logger.debug(f"fetching logs with cursor: {cursor}") result = hosting.get_app_logs( @@ -718,6 +754,15 @@ def app_logs( ) if not isinstance(result, list): logger.warning("Unable to retrieve logs.") + if as_json: + # Kept apart from an empty page: "we could not read them" + # and "there are none" call for different next steps. + print_json({ + "app_id": app_id, + "entries": [], + "cursor": None, + "error": "Unable to retrieve logs.", + }) return if len(result) == 2 and isinstance(result[1], str): cursor = result[1] @@ -726,13 +771,31 @@ def app_logs( cursor = None if not result: logger.warning("No logs found for the specified criteria.") + if as_json: + print_json({ + "app_id": app_id, + "entries": [], + "cursor": cursor, + "error": None, + }) return result.reverse() + if as_json: + # One page per invocation, with the cursor to ask for the next: + # a document is only a document once it is complete, so paging + # is the caller's loop rather than ours. + print_json({ + "app_id": app_id, + "entries": result, + "cursor": cursor, + "error": None, + }) + return for log in result: if pretty: log = pprint.pformat(log, indent=2) logger.info(log) - if not (interactive and follow): + if not following: return from rich.prompt import Prompt @@ -762,19 +825,8 @@ def app_logs( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in JSON format.", -) -@click.option( - "--interactive/--no-interactive", - is_flag=True, - default=True, - help="Whether to list configuration options and ask for confirmation.", -) +@json_option +@interactive_option def list_apps( project_id: str | None, project_name: str | None, @@ -820,7 +872,7 @@ def list_apps( raise click.exceptions.Exit(1) from ex if as_json: - console.print(json.dumps(deployments)) + print_json(deployments) return if deployments: headers = list(deployments[0].keys()) @@ -845,13 +897,8 @@ def list_apps( help="The log level to use.", ) @click.option("--scale-type", help="The type of scaling.") -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def scale_app( app_id: str | None, app_name: str | None, @@ -860,6 +907,7 @@ def scale_app( token: str | None, loglevel: str, scale_type: str | None, + as_json: bool, interactive: bool, ): """Scale an application by changing the VM type or adding/removing regions.""" @@ -920,6 +968,15 @@ def scale_app( hosting.scale_app( app_id=app_id, scale_params=scale_params, client=authenticated_client ) + if as_json: + print_json({ + "app_id": app_id, + "scaled": True, + "vmtype": scale_params.vm_type, + "regions": list(scale_params.regions), + "scale_type": scale_params.type, + }) + return logger.log(log.SUCCESS, "Successfully scaled the app.") except NotAuthenticatedError as err: @@ -945,20 +1002,8 @@ def scale_app( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in JSON format.", -) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def inspect_app( app_id: str | None, token: str | None, @@ -994,7 +1039,7 @@ def inspect_app( app_info = hosting.get_app(app_id=app_id, client=authenticated_client) if as_json: - console.print(json.dumps(app_info)) + print_json(app_info) return if app_info: diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/auth.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/auth.py index e96d80bb79d..f1a5f1d42f7 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/auth.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/auth.py @@ -3,7 +3,6 @@ from __future__ import annotations import hashlib -import json import logging import os import sys @@ -13,6 +12,7 @@ from reflex_cli import constants from reflex_cli.utils import console, log from reflex_cli.utils.exceptions import TokenValidationError +from reflex_cli.utils.output import json_option, print_json logger = logging.getLogger(__name__) @@ -83,13 +83,7 @@ def _resolve_set_token(value: str) -> str: @click.command() @click.option("--token", help="The authentication token.") @_loglevel_option -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) +@json_option def whoami_command(token: str | None, loglevel: str, as_json: bool): """Show which account the Reflex Cloud CLI is authenticating as. @@ -131,7 +125,7 @@ def whoami_command(token: str | None, loglevel: str, as_json: bool): # terminal width, which corrupts JSON and truncates the identifiers this # command exists to hand back. if as_json: - click.echo(json.dumps(identity)) + print_json(identity) return width = max(map(len, identity)) diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/deploy.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/deploy.py index acb514e493c..c9d19ee3322 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/deploy.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/deploy.py @@ -18,6 +18,7 @@ import click from reflex_cli.utils.cli_options import log_options +from reflex_cli.utils.output import interactive_option @click.command(name="deploy") @@ -97,12 +98,7 @@ help="An optional note recorded on this deployment and shown in " "`reflex cloud apps history`.", ) -@click.option( - "--interactive/--no-interactive", - is_flag=True, - default=True, - help="Whether to list configuration options and ask for confirmation.", -) +@interactive_option @click.option( "--envfile", help="The path to an env file to use. Will override any envs set manually.", diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/deployments.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/deployments.py index 08d8ba31106..502f138f156 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/deployments.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/deployments.py @@ -12,6 +12,7 @@ from packaging import version from reflex_cli import constants +from reflex_cli.utils.output import reserve_stdout_for_argv from reflex_cli.v2.apps import apps_cli from reflex_cli.v2.auth import token_command, whoami_command from reflex_cli.v2.gcp import deploy_command as gcp_deploy_command @@ -36,6 +37,11 @@ def hosting_cli(ctx: click.Context) -> None: It provides commands for managing apps, projects, secrets, and VM types/regions. """ + # Before anything below can speak: this callback runs ahead of the + # subcommand's own option parsing, so its --json is not known yet and a + # warning from here would land on stdout in front of the document. + reserve_stdout_for_argv(ctx=ctx) + if _reflex_version is None: ctx.fail("Reflex is not installed. Install it with `pip install reflex`.") if _reflex_version < constants.ReflexHostingCli.MINIMUM_REFLEX_VERSION: diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/gcp.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/gcp.py index ed42669f730..a2be524c314 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/gcp.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/gcp.py @@ -37,6 +37,7 @@ from reflex_cli import constants from reflex_cli.utils import console, log +from reflex_cli.utils.output import interactive_option, json_option, print_json logger = logging.getLogger(__name__) @@ -221,12 +222,8 @@ help="The directory containing the Reflex app. Uploaded to Cloud Build as the build context; the source tree itself is not modified.", ) @click.option("--token", help="The Reflex authentication token.") -@click.option( - "--interactive/--no-interactive", - is_flag=True, - default=True, - help="Whether to prompt before running the deploy script.", -) +@json_option +@interactive_option @click.option( "--dry-run", is_flag=True, @@ -256,6 +253,7 @@ def deploy_command( envs: tuple[str, ...], source_dir: str, token: str | None, + as_json: bool, interactive: bool, dry_run: bool, loglevel: str, @@ -424,6 +422,16 @@ def deploy_command( console.print(env_vars_yaml) console.print("─" * 60) logger.info("Dry run — nothing staged or executed.") + if as_json: + print_json({ + "dry_run": True, + "source_dir": str(source_path), + "deploy_env": deploy_env, + "cloudbuild_yaml": cloudbuild_yaml, + "dockerfile": dockerfile, + "deploy_script": deploy_script, + "env_vars_yaml": env_vars_yaml, + }) return if interactive: @@ -450,6 +458,16 @@ def deploy_command( cwd=source_path, env_overrides=env_overrides, ) + if as_json: + print_json({ + "dry_run": False, + "deployed": exit_code == 0, + "exit_code": exit_code, + "gcp_project": gcp_project, + "region": region, + "service_name": service_name, + "version": version_value, + }) if exit_code != 0: logger.error(f"Deploy script exited with status {exit_code}.") raise click.exceptions.Exit(exit_code) @@ -790,7 +808,7 @@ def _run_deploy_script( cwd=cwd, env=env, check=False, - stdout=sys.stdout, + stdout=sys.stderr if log.is_stdout_reserved() else sys.stdout, stderr=sys.stderr, ) except OSError as ex: diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/project.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/project.py index ddfb2dfc3e1..0f5ab38a4d5 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/project.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/project.py @@ -8,6 +8,7 @@ from reflex_cli import constants from reflex_cli.utils import console, log from reflex_cli.utils.exceptions import NotAuthenticatedError +from reflex_cli.utils.output import interactive_option, json_option, print_json logger = logging.getLogger(__name__) @@ -26,20 +27,8 @@ def project_cli(): default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def create_project( name: str, token: str | None, @@ -64,7 +53,7 @@ def create_project( raise click.exceptions.Exit(1) from err if as_json: - console.print(json.dumps(project)) + print_json(project) return if project: project = [project] @@ -88,18 +77,14 @@ def create_project( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def invite_user_to_project( role: str, user: str, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Invite a user to a project.""" @@ -120,6 +105,9 @@ def invite_user_to_project( if "failed" in result: logger.error(f"Unable to invite user to project: {result}") raise click.exceptions.Exit(1) + if as_json: + print_json({"role_id": role, "user_id": user, "invited": True}) + return logger.log(log.SUCCESS, "Successfully invited user to project.") @@ -133,17 +121,14 @@ def invite_user_to_project( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - is_flag=True, - default=True, - help="Whether to list configuration options and ask for confirmation.", -) +@json_option +@interactive_option def select_project( project_id: str | None, project_name: str | None, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Select a project.""" @@ -183,6 +168,9 @@ def select_project( if "failed" in result: logger.error(result) raise click.exceptions.Exit(1) + if as_json: + print_json({"project_id": project_id, "selected": True, "message": result}) + return logger.log(log.SUCCESS, result) @@ -194,16 +182,12 @@ def select_project( help="The log level to use.", ) @click.option("--token", help="The authentication token.") -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def get_select_project( loglevel: str, token: str | None, + as_json: bool, interactive: bool, ): """Get the currently selected project.""" @@ -219,6 +203,9 @@ def get_select_project( project_details = hosting.get_project( project_id=project, client=authenticated_client ) + if as_json: + print_json({"project_id": project, "name": project_details["name"]}) + return console.print_table( [[project, project_details["name"]]], headers=["Selected Project ID", "Project Name"], @@ -230,6 +217,8 @@ def get_select_project( raise click.exceptions.Exit(1) from None except Exception as e: logger.error(f"Unable to get the currently selected project: {e}") + elif as_json: + print_json({"project_id": None, "name": None}) else: logger.warning( "no selected project. run `reflex cloud project select` to set one." @@ -244,20 +233,8 @@ def get_select_project( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def get_projects( token: str | None, loglevel: str, @@ -275,7 +252,7 @@ def get_projects( ) projects = hosting.get_projects(client=authenticated_client) if as_json: - console.print(json.dumps(projects)) + print_json(projects) return if projects: headers = list(projects[0].keys()) @@ -313,19 +290,8 @@ def get_projects( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) -@click.option( - "--interactive/--no-interactive", - is_flag=True, - default=True, - help="Whether to list configuration options and ask for confirmation.", -) +@json_option +@interactive_option def get_project_roles( project_id: str | None, project_name: str | None, @@ -361,7 +327,7 @@ def get_project_roles( ) if as_json: - console.print(json.dumps(roles)) + print_json(roles) return if roles: headers = list(roles[0].keys()) @@ -392,19 +358,8 @@ def get_project_roles( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) -@click.option( - "--interactive/--no-interactive", - is_flag=True, - default=True, - help="Whether to list configuration options and ask for confirmation.", -) +@json_option +@interactive_option def get_project_role_permissions( role_id: str, project_id: str | None, @@ -440,7 +395,7 @@ def get_project_role_permissions( ) if as_json: - console.print(json.dumps(permissions)) + print_json(permissions) return if permissions: headers = list(permissions[0].keys()) @@ -473,19 +428,8 @@ def get_project_role_permissions( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) -@click.option( - "--interactive/--no-interactive", - is_flag=True, - default=True, - help="Whether to list configuration options and ask for confirmation.", -) +@json_option +@interactive_option def get_project_role_users( project_id: str | None, project_name: str | None, @@ -521,7 +465,7 @@ def get_project_role_users( ) if as_json: - console.print(json.dumps(users)) + print_json(users) return if users: headers = list(users[0].keys()) diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/providers.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/providers.py index 8253244d95d..1f91952270f 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/providers.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/providers.py @@ -9,7 +9,6 @@ from __future__ import annotations -import json import logging from typing import Any @@ -18,6 +17,7 @@ from reflex_cli import constants from reflex_cli.utils import console, log from reflex_cli.utils.exceptions import NotAuthenticatedError +from reflex_cli.utils.output import interactive_option, json_option, print_json logger = logging.getLogger(__name__) @@ -200,20 +200,8 @@ def _connection_row( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def providers_status( org_id: str | None, token: str | None, @@ -245,7 +233,7 @@ def providers_status( raise click.exceptions.Exit(1) from ex if as_json: - console.print(json.dumps(status)) + print_json(status) return configured = status.get("configured") @@ -306,20 +294,8 @@ def providers_status( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def providers_list( org_id: str | None, token: str | None, @@ -381,7 +357,7 @@ def providers_list( runtime_service_accounts = None if as_json: - console.print(json.dumps(connections)) + print_json(connections) return if not connections: console.print( diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/scan.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/scan.py index 4d848d82561..143d3b66f1b 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/scan.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/scan.py @@ -3,7 +3,6 @@ from __future__ import annotations import io -import json import logging import os import time @@ -16,6 +15,7 @@ from reflex_cli import constants from reflex_cli.utils import console, log from reflex_cli.utils.exceptions import NotAuthenticatedError +from reflex_cli.utils.output import interactive_option, json_option, print_json logger = logging.getLogger(__name__) @@ -181,20 +181,8 @@ def _print_violations(result: dict[str, Any]) -> None: default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in JSON format.", -) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def scan_command( directory: Path, token: str | None, @@ -251,7 +239,7 @@ def scan_command( result = payload.get("result") or {} if as_json: - console.print(json.dumps(result)) + print_json(result) else: _print_violations(result) diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/secrets.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/secrets.py index 78434e10951..8289e365e50 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/secrets.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/secrets.py @@ -9,6 +9,7 @@ from reflex_cli import constants from reflex_cli.utils import console, log from reflex_cli.utils.exceptions import NotAuthenticatedError +from reflex_cli.utils.output import interactive_option, json_option, print_json logger = logging.getLogger(__name__) @@ -27,20 +28,8 @@ def secrets_cli(): default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in JSON format.", -) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def get_secrets( app_id: str | None, token: str | None, @@ -77,7 +66,7 @@ def get_secrets( logger.error(secrets) raise click.exceptions.Exit(1) if as_json: - console.print(secrets) + print_json(secrets) return if secrets: headers = ["Keys"] @@ -114,13 +103,8 @@ def get_secrets( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def update_secrets( app_id: str | None, envfile: str | None, @@ -128,6 +112,7 @@ def update_secrets( reboot: bool, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Update secrets for a given application.""" @@ -176,6 +161,14 @@ def update_secrets( hosting.update_secrets( app_id=app_id, secrets=secrets, reboot=reboot, client=authenticated_client ) + if as_json: + # Names only: a value the caller just sent back to them is a secret + # written into a log or a transcript. + print_json({ + "app_id": app_id, + "updated": sorted(secrets), + "rebooted": reboot, + }) except NotAuthenticatedError as err: logger.error("You are not authenticated. Run `reflex login` to authenticate.") raise click.exceptions.Exit(1) from err @@ -196,19 +189,15 @@ def update_secrets( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def delete_secret( app_id: str | None, key: str, token: str | None, reboot: bool, loglevel: str, + as_json: bool, interactive: bool, ): """Delete a secret for a given application.""" @@ -240,6 +229,14 @@ def delete_secret( if "failed" in result: logger.error(result) raise click.exceptions.Exit(1) + if as_json: + print_json({ + "app_id": app_id, + "key": key, + "deleted": True, + "rebooted": reboot, + }) + return logger.log(log.SUCCESS, "Successfully deleted secret.") except NotAuthenticatedError as err: logger.error("You are not authenticated. Run `reflex login` to authenticate.") diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/vmtypes_regions.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/vmtypes_regions.py index cc5b1a3b191..dc191f49f72 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/vmtypes_regions.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/vmtypes_regions.py @@ -1,12 +1,12 @@ """VMTypes and Regions commands for the Reflex Cloud CLI.""" -import json import logging import click from reflex_cli import constants from reflex_cli.utils import console, log +from reflex_cli.utils.output import interactive_option, json_option, print_json logger = logging.getLogger(__name__) @@ -31,16 +31,12 @@ def vm_types_regions_cli(): default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def create_token( name: str, token: str | None, + as_json: bool, interactive: bool, duration: int, loglevel: constants.LogLevel = constants.LogLevel.INFO, @@ -60,6 +56,9 @@ def create_token( token = hosting.create_token( name=name, expiration=duration, client=authenticated_client ) + if as_json: + print_json({"name": name, "token": token, "expires_in_days": duration}) + return logger.log(log.SUCCESS, f"Token: {token}") @@ -71,13 +70,7 @@ def create_token( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) +@json_option def get_vm_types( token: str | None, loglevel: str, @@ -90,7 +83,7 @@ def get_vm_types( vmtypes = hosting.get_vm_types() if as_json: - console.print(json.dumps(vmtypes)) + print_json(vmtypes) return if vmtypes: ordered_vmtpes: list[list[str | float]] = [ @@ -115,13 +108,7 @@ def get_vm_types( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) +@json_option def get_deployment_regions( loglevel: str, as_json: bool, @@ -170,7 +157,7 @@ def get_deployment_regions( list_regions_info = hosting.get_regions() if as_json: - console.print(json.dumps(list_regions_info)) + print_json(list_regions_info) return if list_regions_info: headers = list(list_regions_info[0].keys()) @@ -183,19 +170,22 @@ def get_deployment_regions( @vm_types_regions_cli.command(name="config") @click.option("--token", help="An existing authentication token.") -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def generate_cloud_config( token: str | None = None, + as_json: bool = False, interactive: bool = True, ): """Generate a configuration file for the cloud deployment.""" from reflex_cli.utils import hosting - hosting.generate_config(interactive=interactive, token=token) - console.print("Configuration file generated.") + config_path = hosting.generate_config(interactive=interactive, token=token) + if as_json: + print_json({ + "generated": config_path is not None, + "path": str(config_path.resolve()) if config_path else None, + }) + return + if config_path is not None: + console.print("Configuration file generated.") diff --git a/tests/units/reflex_base/utils/test_log.py b/tests/units/reflex_base/utils/test_log.py index 5826ec47be8..e90c25a7b47 100644 --- a/tests/units/reflex_base/utils/test_log.py +++ b/tests/units/reflex_base/utils/test_log.py @@ -754,3 +754,52 @@ def test_deprecate_json_location_is_user_frame(monkeypatch, capsys): path, _, lineno = location.rpartition(":") assert Path(path).name == Path(__file__).name assert lineno.isdigit() + + +def test_reserve_stdout_moves_log_records_to_stderr(capsys): + """While stdout is reserved, log records render to stderr instead.""" + log.reserve_stdout() + logger.info("progress") + out, err = capsys.readouterr() + assert out == "" + assert "progress" in err + + +def test_reserve_stdout_moves_console_output_to_stderr(capsys): + """Console prints, tables and rules follow the log records.""" + log.reserve_stdout() + console.print("a message") + console.print_table([["one"]], headers=["col"]) + console.rule("a rule") + out, err = capsys.readouterr() + assert out == "" + assert "a message" in err + assert "one" in err + assert "a rule" in err + + +def test_reserve_stdout_moves_json_records_to_stderr(monkeypatch, capsys): + """JSON log records move too: they are still messages, not the document.""" + monkeypatch.setenv("REFLEX_LOG_JSON", "true") + log.configure() + log.reserve_stdout() + logger.info("progress") + out, err = capsys.readouterr() + assert out == "" + assert json.loads(err)["message"] == "progress" + + +def test_releasing_the_reservation_restores_stdout(capsys): + """Human-readable output goes back to stdout once the document is done.""" + log.reserve_stdout() + log.reserve_stdout(False) + logger.info("progress") + out, _ = capsys.readouterr() + assert "progress" in out + + +def test_reset_releases_the_reservation(): + """Teardown cannot leave a later command writing to the wrong stream.""" + log.reserve_stdout() + log._reset() + assert log.is_stdout_reserved() is False diff --git a/tests/units/reflex_cli/utils/test_log.py b/tests/units/reflex_cli/utils/test_log.py index 48c2321810e..e3f0260a3cc 100644 --- a/tests/units/reflex_cli/utils/test_log.py +++ b/tests/units/reflex_cli/utils/test_log.py @@ -5,7 +5,9 @@ import ast import contextlib import importlib +import json import logging +import subprocess import sys from collections.abc import Iterator from enum import Enum @@ -593,3 +595,194 @@ def test_no_command_trusts_the_log_level_it_is_handed(): f"{offenders}. Pass it to console.set_log_level, which resolves a " "foreign reflex's LogLevel by value, and use the result." ) + + +# Each of these runs the shim in a subprocess of its own, the way +# test_deploy.py probes the framework-free import. The obvious in-process +# version -- swapping sys.modules["reflex_base.utils.log"] for a copy missing +# the two names -- leaves the parent package's `log` attribute pointing at the +# stand-in, which is invisible here and destabilized async tests elsewhere in +# the suite. +def _probe(body: str, stdin: str = "") -> subprocess.CompletedProcess[str]: + """Run a snippet against the shim in a clean interpreter. + + Args: + body: The python source to run. + stdin: What to feed the snippet's stdin, for a prompt that reads it. + + Returns: + The finished process, with stdout and stderr captured. + """ + return subprocess.run( + [sys.executable, "-c", body], + input=stdin, + capture_output=True, + text=True, + check=False, + ) + + +_HIDE_THE_RESERVATION = """ +import sys, types +import reflex_base.utils.log as real + +# reflex-base as every published release has it: the three long-standing names, +# and no stdout reservation. +older = types.ModuleType("reflex_base.utils.log") +for attr in dir(real): + if attr not in ("reserve_stdout", "is_stdout_reserved"): + setattr(older, attr, getattr(real, attr)) +sys.modules["reflex_base.utils.log"] = older +""" + + +def test_a_reflex_base_without_the_reservation_is_still_adopted(): + """Two unreleased names must not cost a good reflex-base its whole adoption. + + `reserve_stdout` and `is_stdout_reserved` are newer than the rest of this + shim, so asking for them in the same `try` as SUCCESS / is_json_mode / + set_log_level sent every currently-published reflex-base down the fallback + path -- swapping out its console, its log parenting and its is_json_mode to + acquire a feature it was only ever meant to go without. + """ + result = _probe( + _HIDE_THE_RESERVATION + + """ +from reflex_cli.utils import log +import reflex_cli.utils.console as console + +print(log.HAS_REFLEX_BASE, console.print.__module__, log.reserve_stdout.__module__) +""" + ) + + assert result.returncode == 0, result.stderr + adopted, console_module, reserve_module = result.stdout.split() + assert adopted == "True" + assert console_module == "reflex_base.utils.console" + # The reservation itself degrades, which is the documented trade. + assert reserve_module == "reflex_cli.utils.log" + + +def test_the_reservation_survives_having_no_reflex_base_at_all(): + """`--json` keeps its stdout on a reflex too old to have reflex-base. + + The fallback used to answer False unconditionally, so the reservation was a + no-op: the fallback handler wrote INFO records to stdout in front of the + document, corrupting exactly the output `--json` exists to produce, and + only on old reflex, where nothing would catch it. + """ + result = _probe( + """ +import sys + +class Blocked: + def find_spec(self, name, path=None, target=None): + if name == "reflex_base" or name.startswith("reflex_base."): + raise ImportError(name) + +sys.meta_path.insert(0, Blocked()) + +from reflex_cli.utils import log + +assert not log.HAS_REFLEX_BASE, "reflex_base should be unreachable here" +print(log.is_stdout_reserved(), end=" ") +log.reserve_stdout(True) +print(log.is_stdout_reserved(), end=" ") +log.reserve_stdout(False) +print(log.is_stdout_reserved()) +""" + ) + + assert result.returncode == 0, result.stderr + assert result.stdout.split() == ["False", "True", "False"] + + +def test_the_fallback_moves_human_output_off_a_reserved_stdout(): + """While a document owns stdout, records and prints go to stderr instead.""" + result = _probe( + """ +import sys + +class Blocked: + def find_spec(self, name, path=None, target=None): + if name == "reflex_base" or name.startswith("reflex_base."): + raise ImportError(name) + +sys.meta_path.insert(0, Blocked()) + +import logging +from reflex_cli.constants.base import LogLevel +from reflex_cli.utils import console, log + +console.set_log_level(LogLevel.INFO) +log.reserve_stdout(True) +logging.getLogger("reflex_cli.probe").info("a message for a person") +console.print("a table-ish thing") +print('{"ok": true}') +""" + ) + + assert result.returncode == 0, result.stderr + assert json.loads(result.stdout) == {"ok": True} + assert "a message for a person" in result.stderr + assert "a table-ish thing" in result.stderr + + +def test_the_fallback_asks_its_questions_on_a_reserved_stdout_too(): + """A prompt is the one piece of human output that must not reach stdout. + + It blocks, so a caller parsing the document reads the question as data and + never answers it -- which is how `--json` came to emit one non-JSON line on + the bad-token login fall-through. `console.ask` was reaching Prompt.ask with + no console of its own, so it bypassed the reservation the prints observe. + """ + result = _probe( + """ +import sys + +class Blocked: + def find_spec(self, name, path=None, target=None): + if name == "reflex_base" or name.startswith("reflex_base."): + raise ImportError(name) + +sys.meta_path.insert(0, Blocked()) + +from reflex_cli.utils import console, log + +log.reserve_stdout(True) +console.ask("a question for a person") +print('{"ok": true}') +""", + # Prompt.ask still reads stdin; only where it writes the question moves. + stdin="an answer\n", + ) + + assert result.returncode == 0, result.stderr + assert json.loads(result.stdout) == {"ok": True} + assert "a question for a person" in result.stderr + + +def test_the_reservation_is_asked_for_separately_from_the_rest_of_the_shim(): + """The two import blocks stay two, so one unreleased name cannot widen. + + Read off the source, because merging them back is a one-line edit whose + only symptom is a silent downgrade against a reflex-base that is fine. + """ + source = (Path(reflex_cli.__file__).parent / "utils" / "log.py").read_text() + reservation = {"reserve_stdout", "is_stdout_reserved"} + adoption = {"SUCCESS", "is_json_mode", "set_log_level"} + + for node in ast.walk(ast.parse(source)): + if not isinstance(node, ast.Try): + continue + imported = { + alias.name + for stmt in ast.walk(node) + if isinstance(stmt, ast.ImportFrom) + and (stmt.module or "").startswith("reflex_base") + for alias in stmt.names + } + assert not (imported & reservation and imported & adoption), ( + "the stdout reservation must be imported in a try of its own: " + f"found {sorted(imported)} together" + ) diff --git a/tests/units/reflex_cli/utils/test_output.py b/tests/units/reflex_cli/utils/test_output.py new file mode 100644 index 00000000000..603e1b4d287 --- /dev/null +++ b/tests/units/reflex_cli/utils/test_output.py @@ -0,0 +1,240 @@ +"""Tests for the shared machine-readable output options in reflex_cli.utils.output.""" + +import io +import json + +import click +import pytest +from click.testing import CliRunner +from pytest_mock import MockFixture +from reflex_base.utils import log +from reflex_cli.utils import output + +runner = CliRunner() + + +@pytest.fixture(autouse=True) +def _release_stdout(): + """Release any stdout reservation a test left behind. + + Yields: + None. + """ + yield + log.reserve_stdout(False) + + +@click.command() +@output.json_option +@output.interactive_option +def _probe(as_json: bool, interactive: bool): + """Report the resolved flags and whether stdout was reserved.""" + output.print_json({ + "as_json": as_json, + "interactive": interactive, + "stdout_reserved": log.is_stdout_reserved(), + }) + + +def test_stdout_is_tty_false_for_a_plain_stream(monkeypatch): + """A stream nobody is watching is not a terminal.""" + monkeypatch.setattr("sys.stdout", io.StringIO()) + assert output.stdout_is_tty() is False + + +def test_stdout_is_tty_true_for_a_terminal(monkeypatch): + """A stream that claims to be a terminal is one.""" + stream = io.StringIO() + monkeypatch.setattr(stream, "isatty", lambda: True) + monkeypatch.setattr("sys.stdout", stream) + assert output.stdout_is_tty() is True + + +def test_stdout_is_tty_false_for_a_closed_stream(monkeypatch): + """A closed stream answers False rather than raising.""" + stream = io.StringIO() + stream.close() + monkeypatch.setattr("sys.stdout", stream) + assert output.stdout_is_tty() is False + + +def test_stdout_is_tty_false_when_the_stream_has_no_isatty(monkeypatch): + """A replacement stdout without isatty is not a terminal.""" + monkeypatch.setattr("sys.stdout", object()) + assert output.stdout_is_tty() is False + + +# Every spelling that has to agree with click, since the scan runs before +# click parses and decides where the group callback's own output goes. +_JSON_ARGV_CASES = [ + ([], False), + (["apps", "list"], False), + (["apps", "list", "--json"], True), + (["apps", "list", "-j"], True), + (["apps", "list", "--no-json"], False), + # Last flag wins, in both directions: click parses these as a pair, so a + # membership test that answers "--no-json is present" is wrong about the + # second one. + (["apps", "list", "--json", "--no-json"], False), + (["apps", "list", "--no-json", "--json"], True), + # Short flags combine. + (["apps", "list", "-ij"], True), + (["apps", "list", "-ji"], True), +] + + +@pytest.mark.parametrize(("argv", "expected"), _JSON_ARGV_CASES) +def test_json_requested(argv: list[str], expected: bool): + """A JSON flag on the command line is recognized before click parses it. + + Args: + argv: The arguments to inspect. + expected: Whether they ask for JSON. + """ + assert output.json_requested(argv) is expected + + +@pytest.mark.parametrize(("argv", "expected"), _JSON_ARGV_CASES) +def test_json_requested_agrees_with_click(argv: list[str], expected: bool): + """The pre-parse scan reaches the same answer click's parser does. + + The scan only exists to route output emitted before parsing, so a + disagreement puts a log line on the stdout a document is about to own. + + Args: + argv: The arguments to inspect. + expected: Whether they ask for JSON. + """ + result = runner.invoke(_probe, argv[2:]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["as_json"] is expected + + +def test_reserve_stdout_for_argv_clears_a_stale_reservation(): + """A command line without --json releases a previous reservation.""" + log.reserve_stdout(True) + output.reserve_stdout_for_argv(["apps", "list"]) + assert log.is_stdout_reserved() is False + + +def test_interactive_defaults_off_without_a_terminal(mocker: MockFixture): + """Nothing prompts when stdout is a pipe, a CI job or an agent.""" + mocker.patch("reflex_cli.utils.output.stdout_is_tty", return_value=False) + + result = runner.invoke(_probe, []) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["interactive"] is False + + +def test_interactive_defaults_on_with_a_terminal(mocker: MockFixture): + """A person at a terminal still gets the prompts.""" + mocker.patch("reflex_cli.utils.output.stdout_is_tty", return_value=True) + + result = runner.invoke(_probe, []) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["interactive"] is True + + +@pytest.mark.parametrize( + ("flag", "expected"), + [("--interactive", True), ("-i", True), ("--no-interactive", False)], +) +def test_explicit_interactive_flag_beats_the_terminal( + mocker: MockFixture, flag: str, expected: bool +): + """An explicit flag decides regardless of what stdout is. + + Args: + mocker: The pytest-mock fixture. + flag: The spelling passed on the command line. + expected: The value it resolves to. + """ + mocker.patch("reflex_cli.utils.output.stdout_is_tty", return_value=not expected) + + result = runner.invoke(_probe, [flag]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["interactive"] is expected + + +def test_json_flag_reserves_stdout_before_the_command_runs(): + """The reservation is in place by the time the body can log anything.""" + result = runner.invoke(_probe, ["--json"]) + + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout) + assert payload["as_json"] is True + assert payload["stdout_reserved"] is True + + +def test_no_json_leaves_stdout_unreserved(): + """Without --json, human-readable output keeps stdout.""" + result = runner.invoke(_probe, []) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["stdout_reserved"] is False + + +def test_reservation_is_released_when_the_command_ends(): + """The reservation lasts the command, not the process. + + A CLI process exits and never notices, but an embedding one -- or a second + run in the same interpreter -- would have every later log line writing to + stderr on behalf of a command that finished. + """ + result = runner.invoke(_probe, ["--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["stdout_reserved"] is True + assert log.is_stdout_reserved() is False + + +def test_a_later_command_is_not_reserved_by_an_earlier_one(): + """A plain run after a --json run still writes to stdout.""" + runner.invoke(_probe, ["--json"]) + + result = runner.invoke(_probe, []) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["stdout_reserved"] is False + + +def test_print_json_writes_one_document_while_the_console_writes_to_stderr(): + """The document owns stdout even while messages are being printed. + + This is what makes ``--json`` parseable: a warning from a helper deep in + the call stack would otherwise land in the middle of the document. + """ + from reflex_cli.utils import console + + @click.command() + @output.json_option + def noisy(as_json: bool): + """Print a message and then the document.""" + console.print("a message for a person") + output.print_json({"ok": True}) + + result = runner.invoke(noisy, ["--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == {"ok": True} + assert "a message for a person" in result.stderr + + +def test_print_json_serializes_values_json_cannot(): + """A value without a JSON form is rendered as its string, not an error.""" + + @click.command() + def pathy(): + """Print a payload holding a non-serializable value.""" + from pathlib import Path + + output.print_json({"path": Path("cloud.yml")}) + + result = runner.invoke(pathy, []) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == {"path": "cloud.yml"} diff --git a/tests/units/reflex_cli/v2/test_apps.py b/tests/units/reflex_cli/v2/test_apps.py index 47a0c308dc3..b6bf8b236d5 100644 --- a/tests/units/reflex_cli/v2/test_apps.py +++ b/tests/units/reflex_cli/v2/test_apps.py @@ -89,8 +89,6 @@ def test_app_history_as_json(mocker: MockFixture): } ], ) - mock_console_print = mocker.patch("reflex_cli.utils.console.print") - result = runner.invoke( hosting_cli, ["apps", "history", "test_app_id", "--json"], @@ -103,19 +101,17 @@ def test_app_history_as_json(mocker: MockFixture): token="fake-token", validated_data={"foo": "bar"} ), ) - mock_console_print.assert_called_once_with( - json.dumps([ - { - "id": "deployment1", - "status": "success", - "hostname": "example.com", - "python version": "3.10", - "reflex version": "1.2.3", - "vm type": "small", - "timestamp": "2024-11-29T12:00:00Z", - } - ]) - ) + assert json.loads(result.stdout) == [ + { + "id": "deployment1", + "status": "success", + "hostname": "example.com", + "python version": "3.10", + "reflex version": "1.2.3", + "vm type": "small", + "timestamp": "2024-11-29T12:00:00Z", + } + ] def test_app_history_no_deployments(mocker: MockFixture): @@ -587,7 +583,7 @@ def test_delete_app_success(mocker: MockFixture, caplog: pytest.LogCaptureFixtur ) mock_ask = mocker.patch("reflex_cli.utils.console.ask", return_value="y") - result = runner.invoke(hosting_cli, ["apps", "delete", "app123"]) + result = runner.invoke(hosting_cli, ["apps", "delete", "app123", "--interactive"]) assert result.exit_code == 0, result.output assert mock_get_app.call_count == 2 @@ -648,7 +644,7 @@ def test_delete_app_failure(mocker: MockFixture, caplog: pytest.LogCaptureFixtur ) mock_ask = mocker.patch("reflex_cli.utils.console.ask", return_value="y") - result = runner.invoke(hosting_cli, ["apps", "delete", "app123"]) + result = runner.invoke(hosting_cli, ["apps", "delete", "app123", "--interactive"]) assert result.exit_code == 0, result.output assert mock_get_app.call_count == 2 @@ -739,7 +735,7 @@ def test_delete_app_http_error(mocker: MockFixture, caplog: pytest.LogCaptureFix return_value={"X-API-TOKEN": "fake_token"}, ) - result = runner.invoke(hosting_cli, ["apps", "delete", "app123"]) + result = runner.invoke(hosting_cli, ["apps", "delete", "app123", "--interactive"]) assert result.exit_code == 0, result.output assert mock_get_app.call_count >= 1 @@ -774,7 +770,7 @@ def test_delete_app_confirmation_cancelled( ) mock_ask = mocker.patch("reflex_cli.utils.console.ask", return_value="n") - result = runner.invoke(hosting_cli, ["apps", "delete", "app123"]) + result = runner.invoke(hosting_cli, ["apps", "delete", "app123", "--interactive"]) assert result.exit_code == 0, result.output assert mock_get_app.call_count == 2 @@ -874,7 +870,7 @@ def test_delete_app_get_app_fails_fallback_to_unknown( ) mock_ask = mocker.patch("reflex_cli.utils.console.ask", return_value="y") - result = runner.invoke(hosting_cli, ["apps", "delete", "app123"]) + result = runner.invoke(hosting_cli, ["apps", "delete", "app123", "--interactive"]) assert result.exit_code == 0, result.output assert mock_get_app.call_count == 1 @@ -909,7 +905,9 @@ def test_delete_app_with_app_name_confirmation( ) mock_ask = mocker.patch("reflex_cli.utils.console.ask", return_value="y") - result = runner.invoke(hosting_cli, ["apps", "delete", "--app-name", "my-test-app"]) + result = runner.invoke( + hosting_cli, ["apps", "delete", "--app-name", "my-test-app", "--interactive"] + ) assert result.exit_code == 0, result.output mock_search_app.assert_called_once() @@ -1196,8 +1194,6 @@ def test_list_apps_json_output(mocker: MockFixture): "reflex_cli.utils.hosting.list_apps", return_value=[{"id": "1", "name": "App1"}], ) - mock_print = mocker.patch("reflex_cli.utils.console.print") - result = runner.invoke(hosting_cli, ["apps", "list", "--json"]) assert result.exit_code == 0, result.output @@ -1207,7 +1203,7 @@ def test_list_apps_json_output(mocker: MockFixture): token="fake-token", validated_data={"foo": "bar"} ), ) - mock_print.assert_called_once_with(json.dumps([{"id": "1", "name": "App1"}])) + assert json.loads(result.stdout) == [{"id": "1", "name": "App1"}] def test_list_apps_error(mocker: MockFixture, caplog: pytest.LogCaptureFixture): @@ -1711,7 +1707,7 @@ def test_app_rollback_defaults_to_cancel(mocker: MockFixture): result = runner.invoke( apps_cli, - ["rollback", "dep-1", "--app-id", "app-1"], + ["rollback", "dep-1", "--app-id", "app-1", "--interactive"], input="\n", ) @@ -1839,3 +1835,400 @@ def test_resolve_app_id_explicit_id_wins(mocker: MockFixture): ) search.assert_not_called() read_config.assert_not_called() + + +def _authed(mocker: MockFixture) -> hosting.AuthenticatedClient: + """Patch the client lookup and return the client it hands back. + + Args: + mocker: The pytest-mock fixture. + + Returns: + The authenticated client every command under test will receive. + """ + client = hosting.AuthenticatedClient(token="fake-token", validated_data={}) + mocker.patch( + "reflex_cli.utils.hosting.get_authenticated_client", return_value=client + ) + return client + + +def test_app_logs_does_not_follow_by_default(mocker: MockFixture): + """One page is fetched and the command returns, with nothing to answer. + + Following prompts between pages, and a prompt nobody answers is a command + that never exits -- which is why it is opt-in. + """ + _authed(mocker) + mock_get_app_logs = mocker.patch( + "reflex_cli.utils.hosting.get_app_logs", + return_value=[["log1"], "next-cursor"], + ) + prompt = mocker.patch("rich.prompt.Prompt.ask", return_value="") + + result = runner.invoke(hosting_cli, ["apps", "logs", "app123", "--interactive"]) + + assert result.exit_code == 0, result.output + mock_get_app_logs.assert_called_once() + prompt.assert_not_called() + + +def test_app_logs_follow_needs_a_person_to_answer_the_prompt(mocker: MockFixture): + """--follow is ignored without interactive mode rather than hanging.""" + _authed(mocker) + mock_get_app_logs = mocker.patch( + "reflex_cli.utils.hosting.get_app_logs", + return_value=[["log1"], "next-cursor"], + ) + prompt = mocker.patch("rich.prompt.Prompt.ask", return_value="") + + result = runner.invoke( + hosting_cli, + ["apps", "logs", "app123", "--follow", "true", "--no-interactive"], + ) + + assert result.exit_code == 0, result.output + mock_get_app_logs.assert_called_once() + prompt.assert_not_called() + + +def test_app_logs_follow_pages_when_asked_interactively(mocker: MockFixture): + """Passing --follow at a terminal still walks the pages.""" + _authed(mocker) + mock_get_app_logs = mocker.patch( + "reflex_cli.utils.hosting.get_app_logs", + return_value=[["log1"], "next-cursor"], + ) + prompt = mocker.patch("rich.prompt.Prompt.ask", return_value="exit") + + result = runner.invoke( + hosting_cli, + ["apps", "logs", "app123", "--follow", "true", "--interactive"], + ) + + assert result.exit_code == 0, result.output + mock_get_app_logs.assert_called_once() + prompt.assert_called_once() + + +def test_app_logs_json_output(mocker: MockFixture): + """The page and its next cursor come back as one document.""" + _authed(mocker) + mocker.patch( + "reflex_cli.utils.hosting.get_app_logs", + return_value=[["log1", "log2"], "next-cursor"], + ) + + result = runner.invoke(hosting_cli, ["apps", "logs", "app123", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + # Reversed into chronological order, the same as the rendered form. + "entries": ["log2", "log1"], + "cursor": "next-cursor", + "error": None, + } + + +def test_app_logs_json_output_never_follows(mocker: MockFixture): + """--follow cannot page a document that is only complete once.""" + _authed(mocker) + mock_get_app_logs = mocker.patch( + "reflex_cli.utils.hosting.get_app_logs", + return_value=[["log1"], "next-cursor"], + ) + prompt = mocker.patch("rich.prompt.Prompt.ask", return_value="") + + result = runner.invoke( + hosting_cli, + ["apps", "logs", "app123", "--json", "--follow", "true", "--interactive"], + ) + + assert result.exit_code == 0, result.output + mock_get_app_logs.assert_called_once() + prompt.assert_not_called() + assert json.loads(result.stdout)["cursor"] == "next-cursor" + + +def test_app_logs_json_output_when_empty(mocker: MockFixture): + """No logs is an empty document rather than a warning to parse.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.get_app_logs", return_value=[]) + + result = runner.invoke(hosting_cli, ["apps", "logs", "app123", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "entries": [], + "cursor": None, + "error": None, + } + + +def test_stop_app_json_output(mocker: MockFixture): + """Stopping an app reports the outcome as a document.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.stop_app", return_value="app stopped") + + result = runner.invoke(hosting_cli, ["apps", "stop", "app123", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "stopped": True, + "message": "app stopped", + } + + +def test_stop_app_json_output_on_failure(mocker: MockFixture): + """A refusal is reported in the document, not only in the log.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.stop_app", return_value="stop failed") + + result = runner.invoke(hosting_cli, ["apps", "stop", "app123", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["stopped"] is False + + +def test_start_app_json_output(mocker: MockFixture): + """Starting an app reports the outcome as a document.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.start_app", return_value="app started") + + result = runner.invoke(hosting_cli, ["apps", "start", "app123", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "started": True, + "message": "app started", + } + + +def test_delete_app_json_output(mocker: MockFixture): + """Deleting an app reports the outcome as a document.""" + _authed(mocker) + mocker.patch( + "reflex_cli.utils.hosting.get_app", + return_value={"id": "app123", "name": "test-app"}, + ) + mocker.patch("reflex_cli.utils.hosting.delete_app", return_value="") + + result = runner.invoke(hosting_cli, ["apps", "delete", "app123", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "deleted": True, + "message": "", + } + + +def test_delete_app_json_output_on_failure(mocker: MockFixture): + """A refusal is reported as a failed deletion, not as a deleted app.""" + _authed(mocker) + mocker.patch( + "reflex_cli.utils.hosting.get_app", + return_value={"id": "app123", "name": "test-app"}, + ) + mocker.patch( + "reflex_cli.utils.hosting.delete_app", + return_value="delete app failed: app is deploying", + ) + + result = runner.invoke(hosting_cli, ["apps", "delete", "app123", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "deleted": False, + "message": "delete app failed: app is deploying", + } + + +def test_app_logs_json_output_when_unreadable(mocker: MockFixture): + """Logs that could not be read are distinguishable from none existing.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.get_app_logs", return_value=None) + + result = runner.invoke(hosting_cli, ["apps", "logs", "app123", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "entries": [], + "cursor": None, + "error": "Unable to retrieve logs.", + } + + +def test_delete_app_json_output_when_cancelled(mocker: MockFixture): + """Declining the confirmation is reported rather than left silent.""" + _authed(mocker) + mocker.patch( + "reflex_cli.utils.hosting.get_app", + return_value={"id": "app123", "name": "test-app"}, + ) + delete = mocker.patch("reflex_cli.utils.hosting.delete_app") + mocker.patch("reflex_cli.utils.console.ask", return_value="n") + + result = runner.invoke( + hosting_cli, ["apps", "delete", "app123", "--json", "--interactive"] + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "deleted": False, + "cancelled": True, + } + delete.assert_not_called() + + +def test_app_rollback_json_output(mocker: MockFixture): + """A rollback reports what it rolled back to.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.rollback_deployment", return_value="") + + result = runner.invoke( + hosting_cli, + ["apps", "rollback", "dep-1", "--app-id", "app-1", "--json"], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app-1", + "deployment_id": "dep-1", + "rolled_back": True, + "cancelled": False, + } + + +def test_app_describe_json_output(mocker: MockFixture): + """Setting a changelog note reports the note it set.""" + _authed(mocker) + mocker.patch( + "reflex_cli.utils.hosting.update_deployment_description", return_value="" + ) + + result = runner.invoke( + hosting_cli, + [ + "apps", + "describe", + "dep-1", + "--app-id", + "app-1", + "--description", + "hotfix", + "--json", + ], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app-1", + "deployment_id": "dep-1", + "description": "hotfix", + } + + +def test_deployment_build_logs_json_output(mocker: MockFixture): + """Build logs come back as a field rather than as raw console text.""" + _authed(mocker) + mocker.patch( + "reflex_cli.utils.hosting.get_deployment_build_logs", + return_value="step 1\nstep 2", + ) + + result = runner.invoke(hosting_cli, ["apps", "build-logs", "dep-1", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "deployment_id": "dep-1", + "logs": "step 1\nstep 2", + } + + +def test_deployment_status_json_output(mocker: MockFixture): + """A status read reports the status and whether it is a failure.""" + _authed(mocker) + mocker.patch( + "reflex_cli.utils.hosting.get_deployment_status", return_value="deploying" + ) + + result = runner.invoke(hosting_cli, ["apps", "status", "dep-1", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "deployment_id": "dep-1", + "status": "deploying", + "success": True, + } + + +def test_deployment_status_json_output_while_watching(mocker: MockFixture): + """Watching re-reads the status once it ends, since the watch returns a bool.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.watch_deployment_status", return_value=True) + mocker.patch( + "reflex_cli.utils.hosting.get_deployment_status", + return_value="completed successfully", + ) + + result = runner.invoke( + hosting_cli, ["apps", "status", "dep-1", "--watch", "--json"] + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "deployment_id": "dep-1", + "status": "completed successfully", + "success": True, + } + + +def test_scale_app_json_output(mocker: MockFixture): + """Scaling reports the parameters it applied.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.scale_app") + mocker.patch( + "reflex_cli.core.config.Config.from_yaml_or_toml_or_default", + return_value=Config(), + ) + + result = runner.invoke( + hosting_cli, ["apps", "scale", "app123", "--vmtype", "c1m1", "--json"] + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "scaled": True, + "vmtype": "c1m1", + "regions": [], + "scale_type": "size", + } + + +def test_json_output_keeps_human_messages_off_stdout(mocker: MockFixture): + """A log line from the command body never lands inside the document.""" + _authed(mocker) + mocker.patch( + "reflex_cli.utils.hosting.list_apps", return_value=[{"id": "1", "name": "App1"}] + ) + mocker.patch( + "reflex_cli.utils.hosting.get_selected_project", return_value="project-1" + ) + mocker.patch( + "reflex_cli.utils.hosting.get_project", + return_value={"id": "project-1", "name": "My Project"}, + ) + + result = runner.invoke(hosting_cli, ["apps", "list", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == [{"id": "1", "name": "App1"}] diff --git a/tests/units/reflex_cli/v2/test_auth.py b/tests/units/reflex_cli/v2/test_auth.py index ab00dce41ef..e7f9c8a5e29 100644 --- a/tests/units/reflex_cli/v2/test_auth.py +++ b/tests/units/reflex_cli/v2/test_auth.py @@ -8,7 +8,7 @@ from pytest_mock import MockFixture from reflex_base.utils.log import SUCCESS from reflex_cli import constants -from reflex_cli.utils import hosting +from reflex_cli.utils import console, hosting from reflex_cli.utils.exceptions import TokenAccessDeniedError, TokenValidationError from reflex_cli.v2.auth import token_fingerprint from reflex_cli.v2.deployments import hosting_cli @@ -110,6 +110,30 @@ def test_whoami_json_output_is_exact(mocker: MockFixture): assert json.loads(result.output)["email"] == wide["email"] +def test_whoami_json_reserves_stdout_for_the_document(mocker: MockFixture): + """A log line emitted while `--json` is active never joins the document. + + `whoami` reads its identity through the shared option now, so the + reservation every other cloud command gets applies here too. + """ + mocker.patch( + "reflex_cli.utils.hosting.get_existing_access_token_with_source", + return_value=("valid_token", hosting.TokenSource.CONFIG), + ) + + def _validate(_token: str) -> dict: + console.print("a message for a person") + return dict(VALIDATED_INFO) + + mocker.patch("reflex_cli.utils.hosting.validate_token", side_effect=_validate) + + result = runner.invoke(hosting_cli, ["whoami", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["email"] == VALIDATED_INFO["email"] + assert "a message for a person" in result.stderr + + def test_whoami_prefers_the_token_option(mocker: MockFixture): from_config = mocker.patch( "reflex_cli.utils.hosting.get_existing_access_token_with_source" diff --git a/tests/units/reflex_cli/v2/test_deploy.py b/tests/units/reflex_cli/v2/test_deploy.py index 24860e7f12e..80d9ec85a74 100644 --- a/tests/units/reflex_cli/v2/test_deploy.py +++ b/tests/units/reflex_cli/v2/test_deploy.py @@ -4,10 +4,15 @@ import inspect import subprocess import sys +from unittest import mock import click.testing +import pytest +from pytest_mock import MockFixture +from reflex_base.constants import LogLevel from reflex_cli.v2.deploy import deploy +from reflex import hosting from reflex.reflex import cli EXPECTED_DEPLOY_PARAMS = { @@ -77,6 +82,21 @@ def test_deploy_keeps_log_options(): assert {"--loglevel", "--log-level", "--json"} <= option_names +def test_deploy_interactive_spellings_are_pinned(): + """The exact spellings `--interactive` answers to, including the `-i` alias. + + Adopting the shared option brought `-i` with it, which the parameter-name + check above cannot see: it compares names, and the name did not move. A + later edit to the shared option would change what `reflex deploy` accepts + on the command line, so what it accepts is pinned here rather than left to + be noticed by whoever's script stops working. + """ + interactive = next(param for param in deploy.params if param.name == "interactive") + + assert set(interactive.opts) == {"--interactive", "-i"} + assert set(interactive.secondary_opts) == {"--no-interactive"} + + def test_deploy_uses_only_the_supported_framework_interface(): """The command imports the framework only through `reflex.hosting`. @@ -106,3 +126,77 @@ def test_deploy_help(): result = click.testing.CliRunner().invoke(cli, ["deploy", "--help"]) assert result.exit_code == 0 assert "Deploy the app to the Reflex hosting service." in result.output + + +@pytest.fixture +def driven(mocker: MockFixture) -> mock.MagicMock: + """Stub everything the deploy body reaches for beyond the flags. + + Args: + mocker: The pytest-mock fixture. + + Returns: + The mock standing in for the hosting CLI's own deploy. + """ + mocker.patch("reflex_cli.v2.deployments.check_version") + mocker.patch("reflex_cli.utils.dependency.check_requirements") + mocker.patch( + "reflex.hosting.prepare_deploy", + return_value=hosting.DeployPrep( + app_name="app", loglevel=LogLevel.INFO, ssr=True + ), + ) + return mocker.patch("reflex_cli.v2.cli.deploy") + + +@pytest.mark.parametrize( + ("argv", "tty", "expected"), + [ + ([], False, False), + ([], True, True), + (["--interactive"], False, True), + (["-i"], False, True), + (["--no-interactive"], True, False), + ], +) +def test_deploy_interactive_follows_the_terminal( + mocker: MockFixture, + driven: mock.MagicMock, + argv: list[str], + tty: bool, + expected: bool, +): + """`reflex deploy` resolves --interactive the way every cloud command does. + + A deploy off a TTY prompts for a project, a provider and a browser login, + so inheriting the shared default is what keeps it from hanging in CI. + + Args: + mocker: The pytest-mock fixture. + driven: The stubbed hosting CLI deploy. + argv: The arguments passed on the command line. + tty: Whether stdout is a terminal. + expected: The interactive value the command should resolve to. + """ + mocker.patch("reflex_cli.utils.output.stdout_is_tty", return_value=tty) + + result = click.testing.CliRunner().invoke(deploy, argv) + + assert result.exit_code == 0, result.output + assert driven.call_args.kwargs["interactive"] is expected + + +@pytest.mark.usefixtures("driven") +def test_deploy_off_a_terminal_does_not_check_requirements(mocker: MockFixture): + """The requirements check prompts, so it goes with the prompts. + + Args: + mocker: The pytest-mock fixture. + """ + mocker.patch("reflex_cli.utils.output.stdout_is_tty", return_value=False) + check = mocker.patch("reflex_cli.utils.dependency.check_requirements") + + result = click.testing.CliRunner().invoke(deploy, []) + + assert result.exit_code == 0, result.output + check.assert_not_called() diff --git a/tests/units/reflex_cli/v2/test_gcp.py b/tests/units/reflex_cli/v2/test_gcp.py index bee8c05d741..bf05b1827fc 100644 --- a/tests/units/reflex_cli/v2/test_gcp.py +++ b/tests/units/reflex_cli/v2/test_gcp.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json import os from pathlib import Path from unittest import mock @@ -673,7 +674,15 @@ def test_gcp_deploy_aborts_on_no(mocker: MockFixture, tmp_path: Path): result = runner.invoke( hosting_cli, - ["gcp-standalone", "--gcp", "--gcp-project", "p", "--source", str(tmp_path)], + [ + "gcp-standalone", + "--gcp", + "--gcp-project", + "p", + "--source", + str(tmp_path), + "--interactive", + ], input="n\n", ) @@ -1056,3 +1065,92 @@ def test_deploy_gcp_requires_gcp_project(mocker: MockFixture, tmp_path: Path): @pytest.fixture(autouse=True) def _no_log_level_side_effects(mocker: MockFixture): mocker.patch("reflex_cli.utils.console.set_log_level") + + +def test_gcp_deploy_json_output(mocker: MockFixture, tmp_path: Path): + """A standalone deploy reports where it deployed and whether it worked.""" + _patch_environment(mocker) + _mock_manifest_response(mocker) + + result = runner.invoke( + hosting_cli, + [ + "gcp-standalone", + "--gcp", + "--gcp-project", + "p", + "--region", + "us-central1", + "--service-name", + "svc", + "--version", + "v1", + "--source", + str(tmp_path), + "--json", + ], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "dry_run": False, + "deployed": True, + "exit_code": 0, + "gcp_project": "p", + "region": "us-central1", + "service_name": "svc", + "version": "v1", + } + + +def test_gcp_deploy_json_output_on_dry_run(mocker: MockFixture, tmp_path: Path): + """A dry run hands back what it would have staged, unrendered.""" + run_mock = _patch_environment(mocker) + _mock_manifest_response(mocker) + + result = runner.invoke( + hosting_cli, + [ + "gcp-standalone", + "--gcp", + "--gcp-project", + "p", + "--source", + str(tmp_path), + "--dry-run", + "--json", + ], + ) + + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout) + assert payload["dry_run"] is True + assert payload["dockerfile"] == DOCKERFILE + assert "gcloud builds submit" in payload["deploy_script"] + assert payload["deploy_env"]["GCP_PROJECT"] == "p" + run_mock.assert_not_called() + + +def test_gcp_deploy_json_output_on_script_failure(mocker: MockFixture, tmp_path: Path): + """A failing script still produces a document, alongside the non-zero exit.""" + run_mock = _patch_environment(mocker) + run_mock.return_value = 7 + _mock_manifest_response(mocker) + + result = runner.invoke( + hosting_cli, + [ + "gcp-standalone", + "--gcp", + "--gcp-project", + "p", + "--source", + str(tmp_path), + "--json", + ], + ) + + assert result.exit_code == 7 + payload = json.loads(result.stdout) + assert payload["deployed"] is False + assert payload["exit_code"] == 7 diff --git a/tests/units/reflex_cli/v2/test_project.py b/tests/units/reflex_cli/v2/test_project.py index e8b7bcfca50..7ccc6ff2b30 100644 --- a/tests/units/reflex_cli/v2/test_project.py +++ b/tests/units/reflex_cli/v2/test_project.py @@ -81,8 +81,6 @@ def test_create_project_with_json_output(mocker: MockFixture): token="valid_token", validated_data={"foo": "bar"} ), ) - mock_print = mocker.patch("reflex_cli.utils.console.print") - project_name = "test_project" token = "valid_token" @@ -97,7 +95,7 @@ def test_create_project_with_json_output(mocker: MockFixture): ), ) - mock_print.assert_called_once_with(json.dumps({"name": "test_project", "id": 1})) + assert json.loads(result.stdout) == {"name": "test_project", "id": 1} assert result.exit_code == 0, result.output @@ -509,8 +507,6 @@ def test_get_project_roles_as_json(mocker: MockFixture): {"role": "viewer", "user": "user2@example.com"}, ], ) - mock_console_print = mocker.patch("reflex_cli.utils.console.print") - result = runner.invoke( hosting_cli, ["project", "roles", "--project-id", "test_project_id", "--json"], @@ -523,12 +519,10 @@ def test_get_project_roles_as_json(mocker: MockFixture): token="fake-token", validated_data={"foo": "bar"} ), ) - mock_console_print.assert_called_once_with( - json.dumps([ - {"role": "admin", "user": "user1@example.com"}, - {"role": "viewer", "user": "user2@example.com"}, - ]) - ) + assert json.loads(result.stdout) == [ + {"role": "admin", "user": "user1@example.com"}, + {"role": "viewer", "user": "user2@example.com"}, + ] def test_get_project_roles_empty_roles(mocker: MockFixture): @@ -682,8 +676,6 @@ def test_get_project_role_permissions_as_json(mocker: MockFixture): {"permission": "write", "resource": "resource2"}, ], ) - mock_console_print = mocker.patch("reflex_cli.utils.console.print") - result = runner.invoke( hosting_cli, [ @@ -704,12 +696,10 @@ def test_get_project_role_permissions_as_json(mocker: MockFixture): token="fake-token", validated_data={"foo": "bar"} ), ) - mock_console_print.assert_called_once_with( - json.dumps([ - {"permission": "read", "resource": "resource1"}, - {"permission": "write", "resource": "resource2"}, - ]) - ) + assert json.loads(result.stdout) == [ + {"permission": "read", "resource": "resource1"}, + {"permission": "write", "resource": "resource2"}, + ] def test_get_project_role_permissions_empty_permissions(mocker: MockFixture): @@ -823,8 +813,6 @@ def test_get_project_role_users_as_json(mocker: MockFixture): {"user_id": "user2", "role": "developer"}, ], ) - mock_console_print = mocker.patch("reflex_cli.utils.console.print") - result = runner.invoke( hosting_cli, [ @@ -843,12 +831,10 @@ def test_get_project_role_users_as_json(mocker: MockFixture): token="fake-token", validated_data={"foo": "bar"} ), ) - mock_console_print.assert_called_once_with( - json.dumps([ - {"user_id": "user1", "role": "admin"}, - {"user_id": "user2", "role": "developer"}, - ]) - ) + assert json.loads(result.stdout) == [ + {"user_id": "user1", "role": "admin"}, + {"user_id": "user2", "role": "developer"}, + ] def test_get_project_role_users_empty_users(mocker: MockFixture): @@ -878,3 +864,78 @@ def test_get_project_role_users_empty_users(mocker: MockFixture): ), ) mock_console_print.assert_called_once_with("[]") + + +def _authed(mocker: MockFixture) -> hosting.AuthenticatedClient: + """Patch the client lookup and return the client it hands back. + + Args: + mocker: The pytest-mock fixture. + + Returns: + The authenticated client every command under test will receive. + """ + client = hosting.AuthenticatedClient(token="fake-token", validated_data={}) + mocker.patch( + "reflex_cli.utils.hosting.get_authenticated_client", return_value=client + ) + return client + + +def test_invite_user_to_project_json_output(mocker: MockFixture): + """An invite reports who was invited to what role.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.invite_user_to_project", return_value="ok") + + result = runner.invoke( + hosting_cli, ["project", "invite", "role-1", "user-1", "--json"] + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "role_id": "role-1", + "user_id": "user-1", + "invited": True, + } + + +def test_select_project_json_output(mocker: MockFixture): + """Selecting a project reports the project it selected.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.get_project", return_value={"id": "p1"}) + mocker.patch("reflex_cli.utils.hosting.select_project", return_value="selected p1") + + result = runner.invoke(hosting_cli, ["project", "select", "p1", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "project_id": "p1", + "selected": True, + "message": "selected p1", + } + + +def test_get_selected_project_json_output(mocker: MockFixture): + """The selected project comes back as a document rather than a table.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.get_selected_project", return_value="p1") + mocker.patch( + "reflex_cli.utils.hosting.get_project", + return_value={"id": "p1", "name": "My Project"}, + ) + + result = runner.invoke(hosting_cli, ["project", "selected", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == {"project_id": "p1", "name": "My Project"} + + +def test_get_selected_project_json_output_when_none(mocker: MockFixture): + """No selection is a document saying so, not a warning to parse.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.get_selected_project", return_value=None) + + result = runner.invoke(hosting_cli, ["project", "selected", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == {"project_id": None, "name": None} diff --git a/tests/units/reflex_cli/v2/test_scan.py b/tests/units/reflex_cli/v2/test_scan.py index 69c894ae035..b20ed3f090c 100644 --- a/tests/units/reflex_cli/v2/test_scan.py +++ b/tests/units/reflex_cli/v2/test_scan.py @@ -215,14 +215,12 @@ def test_scan_json_output(mocker: MockFixture, tmp_path: Path): "reflex_cli.utils.hosting.get_security_review", return_value={"job_id": "job123", "status": "complete", "result": _RESULT}, ) - mock_print = mocker.patch("reflex_cli.utils.console.print") - result = runner.invoke( hosting_cli, ["scan", str(tmp_path), "--json", "--fail-on", "none"] ) assert result.exit_code == 0, result.output - mock_print.assert_called_once_with(json.dumps(_RESULT)) + assert json.loads(result.stdout) == _RESULT def test_scan_polls_until_complete(mocker: MockFixture, tmp_path: Path): diff --git a/tests/units/reflex_cli/v2/test_secrets.py b/tests/units/reflex_cli/v2/test_secrets.py index 982f3fd7456..5b85064300f 100644 --- a/tests/units/reflex_cli/v2/test_secrets.py +++ b/tests/units/reflex_cli/v2/test_secrets.py @@ -1,3 +1,4 @@ +import json import logging import tempfile from pathlib import Path @@ -102,8 +103,6 @@ def test_get_secrets_json_output(mocker: MockFixture): token="fake-token", validated_data={"foo": "bar"} ), ) - mock_console_print = mocker.patch("reflex_cli.utils.console.print") - app_id = "app_id" args = ["secrets", "list", app_id, "--json"] @@ -116,10 +115,10 @@ def test_get_secrets_json_output(mocker: MockFixture): token="fake-token", validated_data={"foo": "bar"} ), ) - mock_console_print.assert_called_once_with({ + assert json.loads(result.stdout) == { "secret_key_1": "value1", "secret_key_2": "value2", - }) + } assert result.exit_code == 0, result.output @@ -295,3 +294,102 @@ def test_update_secrets_invalid_env_format(mocker: MockFixture): assert result.exit_code == 1 assert "Invalid env format: should be =." in result.stdout + + +def _authed(mocker: MockFixture) -> hosting.AuthenticatedClient: + """Patch the client lookup and return the client it hands back. + + Args: + mocker: The pytest-mock fixture. + + Returns: + The authenticated client every command under test will receive. + """ + client = hosting.AuthenticatedClient(token="fake-token", validated_data={}) + mocker.patch( + "reflex_cli.utils.hosting.get_authenticated_client", return_value=client + ) + return client + + +def test_update_secrets_json_output(mocker: MockFixture): + """An update reports the names it wrote, never the values.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.update_secrets") + + result = runner.invoke( + hosting_cli, + [ + "secrets", + "update", + "app123", + "--env", + "B=2", + "--env", + "A=1", + "--json", + ], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "updated": ["A", "B"], + "rebooted": False, + } + assert "1" not in json.dumps(json.loads(result.stdout)["updated"]) + + +def test_update_secrets_json_output_keeps_warnings_off_stdout( + mocker: MockFixture, tmp_path: Path +): + """A warning raised on the way through does not break the document. + + Args: + mocker: The pytest-mock fixture. + tmp_path: A temporary directory to hold the env file. + """ + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.update_secrets") + envfile = tmp_path / ".env" + envfile.write_text("A=1\n") + + result = runner.invoke( + hosting_cli, + [ + "secrets", + "update", + "app123", + "--envfile", + str(envfile), + "--env", + "B=2", + "--json", + ], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "updated": ["A"], + "rebooted": False, + } + assert "--envfile is set; ignoring --env" in result.stderr + + +def test_delete_secret_json_output(mocker: MockFixture): + """Deleting a secret reports the key it removed.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.delete_secret", return_value="deleted") + + result = runner.invoke( + hosting_cli, ["secrets", "delete", "app123", "MY_KEY", "--json"] + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "key": "MY_KEY", + "deleted": True, + "rebooted": False, + } diff --git a/tests/units/reflex_cli/v2/test_vmtypes_regions.py b/tests/units/reflex_cli/v2/test_vmtypes_regions.py index b4a050eb6a2..aa24427418b 100644 --- a/tests/units/reflex_cli/v2/test_vmtypes_regions.py +++ b/tests/units/reflex_cli/v2/test_vmtypes_regions.py @@ -47,15 +47,14 @@ def test_get_vm_types_as_json(mocker: MockFixture): {"id": "2", "name": "Medium", "cpu": 4, "ram": 8}, ], ) - mock_console_print = mocker.patch("reflex_cli.utils.console.print") - result = runner.invoke(hosting_cli, ["vmtypes", "--json"]) assert result.exit_code == 0, result.output mock_get_vm_types.assert_called_once() - mock_console_print.assert_called_once_with( - '[{"id": "1", "name": "Small", "cpu": 2, "ram": 4}, {"id": "2", "name": "Medium", "cpu": 4, "ram": 8}]' - ) + assert json.loads(result.stdout) == [ + {"id": "1", "name": "Small", "cpu": 2, "ram": 4}, + {"id": "2", "name": "Medium", "cpu": 4, "ram": 8}, + ] def test_get_vm_types_empty(mocker: MockFixture): @@ -153,18 +152,14 @@ def test_get_deployment_regions_as_json(mocker: MockFixture): {"name": "Stockholm, Sweden", "code": "arn"}, ], ) - mock_print = mocker.patch("reflex_cli.utils.console.print") - result = runner.invoke(hosting_cli, ["regions", "--json"]) assert result.exit_code == 0, result.output mock_get_regions.assert_called_once() - mock_print.assert_called_once_with( - json.dumps([ - {"name": "Amsterdam, Netherlands", "code": "ams"}, - {"name": "Stockholm, Sweden", "code": "arn"}, - ]) - ) + assert json.loads(result.stdout) == [ + {"name": "Amsterdam, Netherlands", "code": "ams"}, + {"name": "Stockholm, Sweden", "code": "arn"}, + ] def test_get_deployment_regions_empty(mocker: MockFixture): @@ -212,3 +207,54 @@ def test_get_deployment_regions_http_error( assert result.exit_code == 0, result.output errors = [r.getMessage() for r in caplog.records if r.levelno == logging.ERROR] assert errors == ["Unable to get regions due to HTTP Error."] + + +def test_create_token_json_output(mocker: MockFixture): + """Minting a token reports it as a field rather than in a log line.""" + mocker.patch( + "reflex_cli.utils.hosting.get_authenticated_client", + return_value=mocker.MagicMock(), + ) + mocker.patch("reflex_cli.utils.hosting.create_token", return_value="tok-1") + + result = runner.invoke(hosting_cli, ["create-token", "ci", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "name": "ci", + "token": "tok-1", + "expires_in_days": 90, + } + + +def test_generate_cloud_config_json_output(mocker: MockFixture, tmp_path): + """Generating a config reports the file it wrote. + + Args: + mocker: The pytest-mock fixture. + tmp_path: A temporary directory standing in for the app root. + """ + written = tmp_path / "cloud.yml" + mocker.patch("reflex_cli.utils.hosting.generate_config", return_value=written) + + result = runner.invoke(hosting_cli, ["config", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "generated": True, + "path": str(written.resolve()), + } + + +def test_generate_cloud_config_json_output_when_nothing_written(mocker: MockFixture): + """A config that already exists is reported as not generated. + + Args: + mocker: The pytest-mock fixture. + """ + mocker.patch("reflex_cli.utils.hosting.generate_config", return_value=None) + + result = runner.invoke(hosting_cli, ["config", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == {"generated": False, "path": None}