From f404205c3fa11ff854ada60fd97e4d1725fb2eb8 Mon Sep 17 00:00:00 2001 From: Martin Xu <15661672+martinxu9@users.noreply.github.com> Date: Tue, 24 Oct 2023 15:35:51 -0700 Subject: [PATCH] CLI improvements (#2026) --- reflex/reflex.py | 81 ++++++++++++++--------------------------- reflex/utils/build.py | 13 +++---- reflex/utils/hosting.py | 36 ++++++++++++------ 3 files changed, 58 insertions(+), 72 deletions(-) diff --git a/reflex/reflex.py b/reflex/reflex.py index 13bbede7e..a8b7d8d2a 100644 --- a/reflex/reflex.py +++ b/reflex/reflex.py @@ -275,10 +275,10 @@ def export( help="The directory to export the zip files to.", show_default=False, ), - backend_exclude_sqlite_db_files: bool = typer.Option( - True, + upload_db_file: bool = typer.Option( + False, help="Whether to exclude sqlite db files when exporting backend.", - show_default=False, + hidden=True, ), loglevel: constants.LogLevel = typer.Option( console._LOG_LEVEL, help="The log level to use." @@ -310,7 +310,7 @@ def export( zip=zipping, zip_dest_dir=zip_dest_dir, deploy_url=config.deploy_url, - backend_exclude_sqlite_db_files=backend_exclude_sqlite_db_files, + upload_db_file=upload_db_file, ) # Post a telemetry event. @@ -431,6 +431,7 @@ def deploy( config.app_name, "--app-name", help="The name of the App to deploy under.", + hidden=True, ), regions: List[str] = typer.Option( list(), @@ -441,20 +442,29 @@ def deploy( envs: List[str] = typer.Option( list(), "--env", - help="The environment variables to set: =. For multiple envs, repeat this option followed by the env name.", + help="The environment variables to set: =. For multiple envs, repeat this option, e.g. --env k1=v2 --env k2=v2.", + ), + cpus: Optional[int] = typer.Option( + None, help="The number of CPUs to allocate.", hidden=True ), - cpus: Optional[int] = typer.Option(None, help="The number of CPUs to allocate."), memory_mb: Optional[int] = typer.Option( - None, help="The amount of memory to allocate." + None, help="The amount of memory to allocate.", hidden=True ), auto_start: Optional[bool] = typer.Option( - True, help="Whether to auto start the instance." + True, + help="Whether to auto start the instance.", + hidden=True, ), auto_stop: Optional[bool] = typer.Option( - True, help="Whether to auto stop the instance." + True, + help="Whether to auto stop the instance.", + hidden=True, ), frontend_hostname: Optional[str] = typer.Option( - None, "--frontend-hostname", help="The hostname of the frontend." + None, + "--frontend-hostname", + help="The hostname of the frontend.", + hidden=True, ), interactive: Optional[bool] = typer.Option( True, @@ -463,14 +473,17 @@ def deploy( with_metrics: Optional[str] = typer.Option( None, help="Setting for metrics scraping for the deployment. Setup required in user code.", + hidden=True, ), with_tracing: Optional[str] = typer.Option( None, help="Setting to export tracing for the deployment. Setup required in user code.", + hidden=True, ), - backend_exclude_sqlite_db_files: bool = typer.Option( - True, - help="Whether to exclude sqlite db files from the backend export.", + upload_db_file: bool = typer.Option( + False, + help="Whether to include local sqlite db files when uploading to hosting service.", + hidden=True, ), loglevel: constants.LogLevel = typer.Option( config.loglevel, help="The log level to use." @@ -558,16 +571,15 @@ def deploy( zipping=True, zip_dest_dir=tmp_dir, loglevel=loglevel, - backend_exclude_sqlite_db_files=backend_exclude_sqlite_db_files, + upload_db_file=upload_db_file, ) except ImportError as ie: console.error( f"Encountered ImportError, did you install all the dependencies? {ie}" ) - raise typer.Exit(1) from ie - finally: if os.path.exists(tmp_dir): shutil.rmtree(tmp_dir) + raise typer.Exit(1) from ie frontend_file_name = constants.ComponentName.FRONTEND.zip() backend_file_name = constants.ComponentName.BACKEND.zip() @@ -750,43 +762,6 @@ def get_deployment_logs( raise typer.Exit(1) from ex -@deployments_cli.command(name="all-logs") -def get_deployment_all_logs( - key: str = typer.Argument(..., help="The name of the deployment."), - loglevel: constants.LogLevel = typer.Option( - config.loglevel, help="The log level to use." - ), -): - """Get the logs for a deployment.""" - console.set_log_level(loglevel) - - console.print("Note: there is a few seconds delay for logs to be available.") - try: - asyncio.run(hosting.get_logs(key, log_type=hosting.LogType.ALL_LOG)) - except Exception as ex: - console.error(f"Unable to get deployment logs due to: {ex}") - raise typer.Exit(1) from ex - - -@deployments_cli.command(name="deploy-logs") -def get_deployment_deploy_logs( - key: str = typer.Argument(..., help="The name of the deployment."), - loglevel: constants.LogLevel = typer.Option( - config.loglevel, help="The log level to use." - ), -): - """Get the logs for a deployment.""" - console.set_log_level(loglevel) - - console.print("Note: there is a few seconds delay for logs to be available.") - try: - # TODO: we need to pass in the from time stamp - asyncio.run(hosting.get_logs(key, log_type=hosting.LogType.DEPLOY_LOG)) - except Exception as ex: - console.error(f"Unable to get deployment logs due to: {ex}") - raise typer.Exit(1) from ex - - @deployments_cli.command(name="regions") def get_deployment_regions( loglevel: constants.LogLevel = typer.Option( diff --git a/reflex/utils/build.py b/reflex/utils/build.py index e00e510e1..e4830ef63 100644 --- a/reflex/utils/build.py +++ b/reflex/utils/build.py @@ -60,7 +60,7 @@ def _zip( target: str, root_dir: str, exclude_venv_dirs: bool, - exclude_sqlite_db_files: bool = True, + upload_db_file: bool = False, dirs_to_exclude: set[str] | None = None, files_to_exclude: set[str] | None = None, ) -> None: @@ -71,7 +71,7 @@ def _zip( target: The target zip file. root_dir: The root directory to zip. exclude_venv_dirs: Whether to exclude venv directories. - exclude_sqlite_db_files: Whether to exclude sqlite db files. + upload_db_file: Whether to include local sqlite db files. dirs_to_exclude: The directories to exclude. files_to_exclude: The files to exclude. @@ -97,8 +97,7 @@ def _zip( files[:] = [ f for f in files - if not f.startswith(".") - and (not exclude_sqlite_db_files or not f.endswith(".db")) + if not f.startswith(".") and (upload_db_file or not f.endswith(".db")) ] files_to_zip += [ os.path.join(root, file) for file in files if file not in files_to_exclude @@ -127,7 +126,7 @@ def export( zip: bool = False, zip_dest_dir: str = os.getcwd(), deploy_url: str | None = None, - backend_exclude_sqlite_db_files: bool = True, + upload_db_file: bool = False, ): """Export the app for deployment. @@ -137,7 +136,7 @@ def export( zip: Whether to zip the app. zip_dest_dir: The destination directory for created zip files (if any) deploy_url: The URL of the deployed app. - backend_exclude_sqlite_db_files: Whether to exclude sqlite db files from the backend zip. + upload_db_file: Whether to include local sqlite db files from the backend zip. """ # Remove the static folder. path_ops.rm(constants.Dirs.WEB_STATIC) @@ -196,7 +195,7 @@ def export( dirs_to_exclude={"assets", "__pycache__"}, files_to_exclude=files_to_exclude, exclude_venv_dirs=True, - exclude_sqlite_db_files=backend_exclude_sqlite_db_files, + upload_db_file=upload_db_file, ) diff --git a/reflex/utils/hosting.py b/reflex/utils/hosting.py index 5d4f194d3..08dfbfa86 100644 --- a/reflex/utils/hosting.py +++ b/reflex/utils/hosting.py @@ -405,6 +405,7 @@ def deploy( with_metrics: A string indicating the metrics endpoint. Raises: + AssertionError: If the request is rejected by the hosting server. Exception: If the operation fails. The exception message is the reason. Returns: @@ -449,13 +450,16 @@ def deploy( # If the server explicitly states bad request, # display a different error if response.status_code == HTTPStatus.BAD_REQUEST: - raise ValueError(response.json()["detail"]) + raise AssertionError("Server rejected this request") response.raise_for_status() response_json = response.json() return DeploymentPostResponse( frontend_url=response_json["frontend_url"], backend_url=response_json["backend_url"], ) + except OSError as oe: + console.debug(f"Client side error related to file operation: {oe}") + raise except httpx.RequestError as re: console.debug(f"Unable to deploy due to request error: {re}") raise Exception("request error") from re @@ -468,9 +472,10 @@ def deploy( except (KeyError, ValidationError) as kve: console.debug(f"Post params or server response format unexpected: {kve}") raise Exception("internal errors") from kve - except ValueError as ve: + except AssertionError as ve: console.debug(f"Unable to deploy due to request error: {ve}") - raise Exception("request error") from ve + # re-raise the error back to the user as client side error + raise except Exception as ex: console.debug(f"Unable to deploy due to internal errors: {ex}.") raise Exception("internal errors") from ex @@ -950,8 +955,12 @@ def interactive_get_deployment_key_from_user_input( key=key_input, frontend_hostname=frontend_hostname, ) - assert pre_deploy_response.reply is not None - assert key_input == pre_deploy_response.reply.key + if ( + pre_deploy_response.reply is None + or key_input != pre_deploy_response.reply.key + ): + # Rejected by server, try again + continue key_candidate = pre_deploy_response.reply.key api_url = pre_deploy_response.reply.api_url deploy_url = pre_deploy_response.reply.deploy_url @@ -1105,13 +1114,16 @@ def get_regions() -> list[dict]: ) response.raise_for_status() response_json = response.json() - assert response_json and isinstance( - response_json, list - ), "Expect server to return a list " - assert not response_json or ( - response_json[0] is not None and isinstance(response_json[0], dict) - ), "Expect return values are dict's" - + if response_json is None or not isinstance(response_json, list): + console.debug("Expect server to return a list ") + return [] + if ( + response_json + and response_json[0] is not None + and isinstance(response_json[0], dict) + ): + console.debug("Expect return values are dict's") + return [] return response_json except Exception as ex: console.debug(f"Unable to get regions due to {ex}.")