From a5a3589e1f1cb627684fdb3755a64e3016bc62a4 Mon Sep 17 00:00:00 2001 From: Martin Xu <15661672+martinxu9@users.noreply.github.com> Date: Mon, 30 Oct 2023 14:04:40 -0700 Subject: [PATCH 1/5] use http endpoint to return deploy milestones --- reflex/reflex.py | 9 ++-- reflex/utils/hosting.py | 104 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 104 insertions(+), 9 deletions(-) diff --git a/reflex/reflex.py b/reflex/reflex.py index dca9a16506..d9361b9e69 100644 --- a/reflex/reflex.py +++ b/reflex/reflex.py @@ -669,9 +669,10 @@ def deploy( console.print("Waiting for server to report progress ...") # Display the key events such as build, deploy, etc - server_report_deploy_success = asyncio.get_event_loop().run_until_complete( - hosting.display_deploy_milestones(key, from_iso_timestamp=deploy_requested_at) + server_report_deploy_success = hosting.poll_deploy_milestones( + key, from_iso_timestamp=deploy_requested_at ) + if not server_report_deploy_success: console.error("Hosting server reports failure.") console.error( @@ -797,7 +798,7 @@ def get_deployment_status( status = hosting.get_deployment_status(key) # TODO: refactor all these tabulate calls - status.backend.updated_at = hosting.convert_to_local_time( + status.backend.updated_at = hosting.convert_to_local_time_str( status.backend.updated_at or "N/A" ) backend_status = status.backend.dict(exclude_none=True) @@ -806,7 +807,7 @@ def get_deployment_status( console.print(tabulate([table], headers=headers)) # Add a new line in console console.print("\n") - status.frontend.updated_at = hosting.convert_to_local_time( + status.frontend.updated_at = hosting.convert_to_local_time_str( status.frontend.updated_at or "N/A" ) frontend_status = status.frontend.dict(exclude_none=True) diff --git a/reflex/utils/hosting.py b/reflex/utils/hosting.py index 1e05ea4979..cfd171b0c9 100644 --- a/reflex/utils/hosting.py +++ b/reflex/utils/hosting.py @@ -9,7 +9,7 @@ import time import uuid import webbrowser -from datetime import datetime +from datetime import datetime, timedelta from http import HTTPStatus from typing import List, Optional @@ -41,6 +41,8 @@ GET_REGIONS_ENDPOINT = f"{config.cp_backend_url}/deployments/regions" # Websocket endpoint to stream logs of a deployment DEPLOYMENT_LOGS_ENDPOINT = f'{config.cp_backend_url.replace("http", "ws")}/deployments' +# The HTTP endpoint to fetch logs of a deployment +POST_DEPLOYMENT_LOGS_ENDPOINT = f"{config.cp_backend_url}/deployments/logs" # Expected server response time to new deployment request. In seconds. DEPLOYMENT_PICKUP_DELAY = 30 # End of deployment workflow message. Used to determine if it is the last message from server. @@ -726,7 +728,22 @@ def get_deployment_status(key: str) -> DeploymentStatusResponse: raise Exception("internal errors") from ex -def convert_to_local_time(iso_timestamp: str) -> str: +def convert_to_local_time_with_tz(iso_timestamp: str) -> datetime | None: + """Helper function to convert the iso timestamp to local time. + + Args: + iso_timestamp: The iso timestamp to convert. + + Returns: + The converted timestamp with timezone. + """ + try: + return datetime.fromisoformat(iso_timestamp).astimezone() + except Exception: + return None + + +def convert_to_local_time_str(iso_timestamp: str) -> str: """Convert the iso timestamp to local time. Args: @@ -736,7 +753,9 @@ def convert_to_local_time(iso_timestamp: str) -> str: The converted timestamp string. """ try: - local_dt = datetime.fromisoformat(iso_timestamp).astimezone() + local_dt = convert_to_local_time_with_tz(iso_timestamp) + if local_dt is None: + return iso_timestamp return local_dt.strftime("%Y-%m-%d %H:%M:%S.%f %Z") except Exception as ex: console.error(f"Unable to convert iso timestamp {iso_timestamp} due to {ex}.") @@ -798,7 +817,7 @@ async def get_logs( if v is None: row_to_print[k] = str(v) elif k == "timestamp": - row_to_print[k] = convert_to_local_time(v) + row_to_print[k] = convert_to_local_time_str(v) else: row_to_print[k] = v print(" | ".join(row_to_print.values())) @@ -1006,6 +1025,81 @@ def log_out_on_browser(): ) +def poll_deploy_milestones(key: str, from_iso_timestamp: datetime) -> bool: + """Periodically poll the hosting server for deploy milestones. + + Args: + key: The deployment key. + from_iso_timestamp: The timestamp of the deployment request time, this helps with the milestone query. + + Raises: + ValueError: If a non-empty key is not provided. + Exception: If the user is not authenticated. + + Returns: + False if server reports back failure, True otherwise. + """ + if not key: + raise ValueError("Non-empty key is required for querying deploy status.") + if not (token := requires_authenticated()): + raise Exception("not authenticated") + + for _ in range(DEPLOYMENT_EVENT_MESSAGES_RETRIES): + try: + response = httpx.post( + POST_DEPLOYMENT_LOGS_ENDPOINT, + json={ + "key": key, + "log_type": LogType.DEPLOY_LOG.value, + "from_iso_timestamp": from_iso_timestamp.astimezone().isoformat(), + }, + headers=authorization_header(token), + ) + response.raise_for_status() + # The return is expected to be a list of dicts + response_json = response.json() + for row in response_json: + console.print( + " | ".join( + [ + convert_to_local_time_str(row["timestamp"]), + row["message"], + ] + ) + ) + # update the from timestamp to the last timestamp of received message + if ( + maybe_timestamp := convert_to_local_time_with_tz(row["timestamp"]) + ) is not None: + console.debug( + f"Updating from {from_iso_timestamp} to {maybe_timestamp}" + ) + # Add a small delta so does not poll the same logs + from_iso_timestamp = maybe_timestamp + timedelta(microseconds=1e5) + else: + console.error(f"Unable to parse timestamp {row['timestamp']}") + raise ValueError + server_message = row["message"].lower() + if "fail" in server_message: + console.debug( + "Received failure message, stop event message streaming" + ) + return False + if any(msg in server_message for msg in END_OF_DEPLOYMENT_MESSAGES): + console.debug( + "Received end of deployment message, stop event message streaming" + ) + return True + time.sleep(1) + except httpx.HTTPError as he: + # This includes HTTP server and client error + console.debug(f"Unable to get more deployment events due to {he}.") + except Exception as ex: + console.warn(f"Unable to parse server response due to {ex}.") + + return False + + async def display_deploy_milestones(key: str, from_iso_timestamp: datetime) -> bool: """Display the deploy milestone messages reported back from the hosting server. @@ -1039,7 +1133,7 @@ async def display_deploy_milestones(key: str, from_iso_timestamp: datetime) -> b console.print( " | ".join( [ - convert_to_local_time(row_json["timestamp"]), + convert_to_local_time_str(row_json["timestamp"]), row_json["message"], ] ) From c292c7132a509cf3f70e1c50e094324987d436e6 Mon Sep 17 00:00:00 2001 From: Martin Xu <15661672+martinxu9@users.noreply.github.com> Date: Mon, 30 Oct 2023 14:10:18 -0700 Subject: [PATCH 2/5] fix test --- tests/test_reflex.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_reflex.py b/tests/test_reflex.py index 44d3980d3a..a0667e3e62 100644 --- a/tests/test_reflex.py +++ b/tests/test_reflex.py @@ -118,7 +118,7 @@ def test_deploy_non_interactive_success( ), ) mocker.patch("reflex.utils.hosting.wait_for_server_to_pick_up_request") - mocker.patch("reflex.utils.hosting.display_deploy_milestones") + mocker.patch("reflex.utils.hosting.poll_deploy_milestones") mocker.patch("reflex.utils.hosting.poll_backend", return_value=True) mocker.patch("reflex.utils.hosting.poll_frontend", return_value=True) # TODO: typer option default not working in test for app name @@ -351,7 +351,7 @@ def test_deploy_interactive( ), ) mocker.patch("reflex.utils.hosting.wait_for_server_to_pick_up_request") - mocker.patch("reflex.utils.hosting.display_deploy_milestones") + mocker.patch("reflex.utils.hosting.poll_deploy_milestones") mocker.patch("reflex.utils.hosting.poll_backend", return_value=True) mocker.patch("reflex.utils.hosting.poll_frontend", return_value=True) From b3edc40237213d76d7a7fdefb2b032f9741507db Mon Sep 17 00:00:00 2001 From: Martin Xu <15661672+martinxu9@users.noreply.github.com> Date: Mon, 30 Oct 2023 14:40:07 -0700 Subject: [PATCH 3/5] distinguish between explicit failure and not receving it in deploy milestones --- reflex/reflex.py | 5 ++++- reflex/utils/hosting.py | 6 ++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/reflex/reflex.py b/reflex/reflex.py index d9361b9e69..76f33ff7b1 100644 --- a/reflex/reflex.py +++ b/reflex/reflex.py @@ -673,7 +673,10 @@ def deploy( key, from_iso_timestamp=deploy_requested_at ) - if not server_report_deploy_success: + if server_report_deploy_success is None: + console.warn("Hosting server timed out.") + console.warn("The deployment may still be in progress. Proceeding ...") + elif not server_report_deploy_success: console.error("Hosting server reports failure.") console.error( f"Check the server logs using `reflex deployments build-logs {key}`" diff --git a/reflex/utils/hosting.py b/reflex/utils/hosting.py index cfd171b0c9..f034de13ae 100644 --- a/reflex/utils/hosting.py +++ b/reflex/utils/hosting.py @@ -1025,7 +1025,7 @@ def log_out_on_browser(): ) -def poll_deploy_milestones(key: str, from_iso_timestamp: datetime) -> bool: +def poll_deploy_milestones(key: str, from_iso_timestamp: datetime) -> bool | None: """Periodically poll the hosting server for deploy milestones. Args: @@ -1037,7 +1037,7 @@ def poll_deploy_milestones(key: str, from_iso_timestamp: datetime) -> bool: Exception: If the user is not authenticated. Returns: - False if server reports back failure, True otherwise. + False if server reports back failure, True otherwise. None if do not receive the end of deployment message. """ if not key: raise ValueError("Non-empty key is required for querying deploy status.") @@ -1097,8 +1097,6 @@ def poll_deploy_milestones(key: str, from_iso_timestamp: datetime) -> bool: except Exception as ex: console.warn(f"Unable to parse server response due to {ex}.") - return False - async def display_deploy_milestones(key: str, from_iso_timestamp: datetime) -> bool: """Display the deploy milestone messages reported back from the hosting server. From 7520a6fb22a8f1caf683f580dde05c82a027665a Mon Sep 17 00:00:00 2001 From: Martin Xu <15661672+martinxu9@users.noreply.github.com> Date: Thu, 2 Nov 2023 15:14:51 -0700 Subject: [PATCH 4/5] review comment --- reflex/utils/hosting.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/reflex/utils/hosting.py b/reflex/utils/hosting.py index f034de13ae..5f23903edf 100644 --- a/reflex/utils/hosting.py +++ b/reflex/utils/hosting.py @@ -739,7 +739,8 @@ def convert_to_local_time_with_tz(iso_timestamp: str) -> datetime | None: """ try: return datetime.fromisoformat(iso_timestamp).astimezone() - except Exception: + except (TypeError, ValueError) as ex: + console.error(f"Unable to convert iso timestamp {iso_timestamp} due to {ex}.") return None @@ -752,14 +753,9 @@ def convert_to_local_time_str(iso_timestamp: str) -> str: Returns: The converted timestamp string. """ - try: - local_dt = convert_to_local_time_with_tz(iso_timestamp) - if local_dt is None: - return iso_timestamp - return local_dt.strftime("%Y-%m-%d %H:%M:%S.%f %Z") - except Exception as ex: - console.error(f"Unable to convert iso timestamp {iso_timestamp} due to {ex}.") + if (local_dt := convert_to_local_time_with_tz(iso_timestamp)) is None: return iso_timestamp + return local_dt.strftime("%Y-%m-%d %H:%M:%S.%f %Z") class LogType(str, enum.Enum): @@ -1077,8 +1073,7 @@ def poll_deploy_milestones(key: str, from_iso_timestamp: datetime) -> bool | Non # Add a small delta so does not poll the same logs from_iso_timestamp = maybe_timestamp + timedelta(microseconds=1e5) else: - console.error(f"Unable to parse timestamp {row['timestamp']}") - raise ValueError + console.warn(f"Unable to parse timestamp {row['timestamp']}") server_message = row["message"].lower() if "fail" in server_message: console.debug( From 14b64cca6801067a27817b3f7553f642f3a7af76 Mon Sep 17 00:00:00 2001 From: Martin Xu <15661672+martinxu9@users.noreply.github.com> Date: Thu, 2 Nov 2023 15:20:25 -0700 Subject: [PATCH 5/5] increase retry --- reflex/utils/hosting.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reflex/utils/hosting.py b/reflex/utils/hosting.py index 5f23903edf..0bd506cff0 100644 --- a/reflex/utils/hosting.py +++ b/reflex/utils/hosting.py @@ -48,7 +48,7 @@ # End of deployment workflow message. Used to determine if it is the last message from server. END_OF_DEPLOYMENT_MESSAGES = ["deploy success"] # How many iterations to try and print the deployment event messages from server during deployment. -DEPLOYMENT_EVENT_MESSAGES_RETRIES = 90 +DEPLOYMENT_EVENT_MESSAGES_RETRIES = 120 # Timeout limit for http requests HTTP_REQUEST_TIMEOUT = 60 # seconds