Переглянути джерело

add region check upfront when user deploys interactively (#2030)

Martin Xu 1 рік тому
батько
коміт
fe01f0cf11

+ 2 - 2
reflex/constants/config.py

@@ -2,7 +2,7 @@
 import os
 from types import SimpleNamespace
 
-from reflex.constants.base import Dirs
+from reflex.constants.base import Dirs, Reflex
 
 from .compiler import Ext
 
@@ -47,7 +47,7 @@ class RequirementsTxt(SimpleNamespace):
     # The requirements.txt file.
     FILE = "requirements.txt"
     # The partial text used to form requirement that pins a reflex version
-    DEFAULTS_STUB = "reflex=="
+    DEFAULTS_STUB = f"{Reflex.MODULE_NAME}=="
 
 
 # The deployment URL.

+ 29 - 9
reflex/reflex.py

@@ -494,18 +494,22 @@ def deploy(
     console.set_log_level(loglevel)
 
     if not interactive and not key:
-        console.error("Please provide a deployment key when not in interactive mode.")
+        console.error(
+            "Please provide a name for the deployed instance when not in interactive mode."
+        )
         raise typer.Exit(1)
 
-    try:
-        hosting.check_requirements_txt_exist()
-    except Exception as ex:
+    if not hosting.check_requirements_for_non_reflex_packages():
+        console.ask(
+            f"Make sure {constants.RequirementsTxt.FILE} has all the dependencies. Enter to proceed"
+        )
+    if not hosting.check_requirements_txt_exist():
         console.error(f"{constants.RequirementsTxt.FILE} required for deployment")
-        raise typer.Exit(1) from ex
+        raise typer.Exit(1)
 
     # Check if we are set up.
     prerequisites.check_initialized(frontend=True)
-
+    enabled_regions = None
     try:
         # Send a request to server to obtain necessary information
         # in preparation of a deployment. For example,
@@ -515,6 +519,10 @@ def deploy(
         pre_deploy_response = hosting.prepare_deploy(
             app_name, key=key, frontend_hostname=frontend_hostname
         )
+        # Note: we likely won't need to fetch this twice
+        if pre_deploy_response.enabled_regions is not None:
+            enabled_regions = pre_deploy_response.enabled_regions
+
     except Exception as ex:
         console.error(f"Unable to prepare deployment due to: {ex}")
         raise typer.Exit(1) from ex
@@ -544,9 +552,19 @@ def deploy(
         key = key_candidate
 
         # Then CP needs to know the user's location, which requires user permission
-        region_input = console.ask(
-            "Region to deploy to", default=regions[0] if regions else "sjc"
-        )
+        console.debug(f"{enabled_regions=}")
+        while True:
+            region_input = console.ask(
+                "Region to deploy to", default=regions[0] if regions else "sjc"
+            )
+
+            if enabled_regions is None or region_input in enabled_regions:
+                break
+            else:
+                console.warn(
+                    f"{region_input} is not a valid region. Must be one of {enabled_regions}"
+                )
+                console.warn("Run `reflex deploymemts regions` to see details.")
         regions = regions or [region_input]
 
         # process the envs
@@ -557,6 +575,8 @@ def deploy(
     if not key or not regions or not app_name or not app_prefix or not api_url:
         console.error("Please provide all the required parameters.")
         raise typer.Exit(1)
+    # Note: if the user uses --no-interactive mode, there was no prepare_deploy call
+    # so we do not check the regions until the call to hosting server
 
     processed_envs = hosting.process_envs(envs) if envs else None
 

+ 37 - 12
reflex/utils/hosting.py

@@ -223,6 +223,7 @@ class DeploymentPrepareResponse(Base):
     # This is for a new deployment, user has not deployed this app before.
     # The server returns key suggestion based on the app name.
     suggestion: Optional[DeploymentPrepInfo] = None
+    enabled_regions: Optional[List[str]] = None
 
     @root_validator(pre=True)
     def ensure_at_least_one_deploy_params(cls, values):
@@ -303,6 +304,7 @@ def prepare_deploy(
             reply=response_json["reply"],
             suggestion=response_json["suggestion"],
             existing=response_json["existing"],
+            enabled_regions=response_json.get("enabled_regions"),
         )
     except httpx.RequestError as re:
         console.debug(f"Unable to prepare launch due to {re}.")
@@ -450,7 +452,7 @@ def deploy(
         # If the server explicitly states bad request,
         # display a different error
         if response.status_code == HTTPStatus.BAD_REQUEST:
-            raise AssertionError("Server rejected this request")
+            raise AssertionError(f"Server rejected this request: {response.text}")
         response.raise_for_status()
         response_json = response.json()
         return DeploymentPostResponse(
@@ -843,16 +845,37 @@ async def get_logs(
         )
 
 
-def check_requirements_txt_exist():
-    """Check if requirements.txt exists in the current directory.
+def check_requirements_txt_exist() -> bool:
+    """Check if requirements.txt exists in the top level app directory.
 
-    Raises:
-        Exception: If the requirements.txt does not exist.
+    Returns:
+        True if requirements.txt exists, False otherwise.
     """
-    if not os.path.exists(constants.RequirementsTxt.FILE):
-        raise Exception(
-            f"Unable to find {constants.RequirementsTxt.FILE} in the current directory."
-        )
+    return os.path.exists(constants.RequirementsTxt.FILE)
+
+
+def check_requirements_for_non_reflex_packages() -> bool:
+    """Check the requirements.txt file for packages other than reflex.
+
+    Returns:
+        True if packages other than reflex are found, False otherwise.
+    """
+    if not check_requirements_txt_exist():
+        return False
+    try:
+        with open(constants.RequirementsTxt.FILE) as fp:
+            for req_line in fp.readlines():
+                package_name = re.search(r"^([^=<>!~]+)", req_line.lstrip())
+                # If we find a package that is not reflex
+                if (
+                    package_name
+                    and package_name.group(1) != constants.Reflex.MODULE_NAME
+                ):
+                    return True
+    except Exception as ex:
+        console.warn(f"Unable to scan requirements.txt for dependencies due to {ex}")
+
+    return False
 
 
 def authenticate_on_browser(
@@ -948,7 +971,9 @@ def interactive_get_deployment_key_from_user_input(
         deploy_url = suggestion.deploy_url
 
         # If user takes the suggestion, we will use the suggested key and proceed
-        while key_input := console.ask(f"Name of deployment", default=key_candidate):
+        while key_input := console.ask(
+            f"Choose a name for your deployed app", default=key_candidate
+        ):
             try:
                 pre_deploy_response = prepare_deploy(
                     app_name,
@@ -1083,7 +1108,7 @@ def interactive_prompt_for_envs() -> list[str]:
     envs_finished = False
     env_count = 1
     env_key_prompt = f" * env-{env_count} name (enter to skip)"
-    console.print("Environment variables ...")
+    console.print("Environment variables for your production App ...")
     while not envs_finished:
         env_key = console.ask(env_key_prompt)
         if not env_key:
@@ -1120,7 +1145,7 @@ def get_regions() -> list[dict]:
         if (
             response_json
             and response_json[0] is not None
-            and isinstance(response_json[0], dict)
+            and not isinstance(response_json[0], dict)
         ):
             console.debug("Expect return values are dict's")
             return []

+ 11 - 0
tests/test_reflex.py

@@ -98,6 +98,7 @@ def test_deploy_non_interactive_prepare_failed(
 def test_deploy_non_interactive_success(
     mocker, setup_env_authentication, optional_args, values
 ):
+    mocker.patch("reflex.utils.console.ask")
     app_prefix = "fake-prefix"
     mocker.patch(
         "reflex.utils.hosting.prepare_deploy",
@@ -201,6 +202,7 @@ def test_deploy_interactive_prepare_failed(
                     key=get_suggested_key(),
                 ),
                 existing=None,
+                enabled_regions=["sjc"],
             ),
             ["sjc"],
             [],
@@ -220,6 +222,7 @@ def test_deploy_interactive_prepare_failed(
                     key=get_suggested_key(),
                 ),
                 existing=None,
+                enabled_regions=["sjc"],
             ),
             ["sjc"],
             ["k1=v1", "k2=v2"],
@@ -239,6 +242,7 @@ def test_deploy_interactive_prepare_failed(
                     key=get_suggested_key(),
                 ),
                 existing=None,
+                enabled_regions=["sjc"],
             ),
             ["sjc"],
             [],
@@ -258,6 +262,7 @@ def test_deploy_interactive_prepare_failed(
                     key=get_suggested_key(),
                 ),
                 existing=None,
+                enabled_regions=["sjc"],
             ),
             ["sjc"],
             ["k1=v1", "k3=v3"],
@@ -279,6 +284,7 @@ def test_deploy_interactive_prepare_failed(
                     )
                 ),
                 suggestion=None,
+                enabled_regions=["sjc"],
             ),
             ["sjc"],
             [],
@@ -300,6 +306,7 @@ def test_deploy_interactive_prepare_failed(
                     )
                 ),
                 suggestion=None,
+                enabled_regions=["sjc"],
             ),
             ["sjc"],
             ["k4=v4"],
@@ -319,6 +326,10 @@ def test_deploy_interactive(
     expected_key,
     args_patch,
 ):
+    mocker.patch(
+        "reflex.utils.hosting.check_requirements_for_non_reflex_packages",
+        return_value=True,
+    )
     mocker.patch(
         "reflex.utils.hosting.prepare_deploy",
         return_value=prepare_responses,

+ 14 - 0
tests/utils/test_hosting.py

@@ -353,3 +353,17 @@ def test_process_envs():
 def test_interactive_prompt_for_envs(mocker, inputs, expected):
     mocker.patch("reflex.utils.console.ask", side_effect=inputs)
     assert hosting.interactive_prompt_for_envs() == expected
+
+
+def test_requirements_txt_only_contains_reflex(mocker):
+    mocker.patch("reflex.utils.hosting.check_requirements_txt_exist", return_value=True)
+    mocker.patch("builtins.open", mock_open(read_data="\nreflex=1.2.3\n\n"))
+    assert hosting.check_requirements_for_non_reflex_packages() is False
+
+
+def test_requirements_txt_only_contains_other_packages(mocker):
+    mocker.patch("reflex.utils.hosting.check_requirements_txt_exist", return_value=True)
+    mocker.patch(
+        "builtins.open", mock_open(read_data="\nreflex=1.2.3\n\npynonexist=3.2.1")
+    )
+    assert hosting.check_requirements_for_non_reflex_packages() is True