From 7f22a2e4223e37f4534ba8b917573d9de6947536 Mon Sep 17 00:00:00 2001 From: Benedikt Bartscher Date: Thu, 7 Nov 2024 23:24:52 +0100 Subject: [PATCH] wip env var prefix, rx.Config tbd --- reflex/app.py | 5 +- reflex/config.py | 60 ++++++++++++++----- reflex/custom_components/custom_components.py | 4 +- reflex/model.py | 14 ++--- reflex/reflex.py | 2 +- reflex/testing.py | 6 +- reflex/utils/net.py | 2 +- reflex/utils/prerequisites.py | 9 ++- reflex/utils/registry.py | 2 +- tests/integration/conftest.py | 7 ++- tests/units/test_config.py | 11 ++++ 11 files changed, 85 insertions(+), 37 deletions(-) diff --git a/reflex/app.py b/reflex/app.py index a8a6e69c6..48dbb4158 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -503,7 +503,10 @@ class App(MiddlewareMixin, LifespanMixin): # Check if the route given is valid verify_route_validity(route) - if route in self.unevaluated_pages and environment.RELOAD_CONFIG.is_set(): + if ( + route in self.unevaluated_pages + and environment.REFLEX_RELOAD_CONFIG.is_set() + ): # when the app is reloaded(typically for app harness tests), we should maintain # the latest render function of a route.This applies typically to decorated pages # since they are only added when app._compile is called. diff --git a/reflex/config.py b/reflex/config.py index 049cc2e83..0a0bab991 100644 --- a/reflex/config.py +++ b/reflex/config.py @@ -319,18 +319,21 @@ class EnvVar(Generic[T]): name: str default: Any type_: T + noprefix: bool - def __init__(self, name: str, default: Any, type_: T) -> None: + def __init__(self, name: str, default: Any, type_: T, noprefix: bool) -> None: """Initialize the environment variable. Args: name: The environment variable name. default: The default value. type_: The type of the value. + noprefix: Whether to allow non-prefixed environment variables (deprecated). """ self.name = name self.default = default self.type_ = type_ + self.noprefix = noprefix def interpret(self, value: str) -> T: """Interpret the environment variable value. @@ -350,6 +353,21 @@ class EnvVar(Generic[T]): The environment variable value. """ env_value = os.getenv(self.name, None) + if self.noprefix: + deprecated_env = self.name.removeprefix("REFLEX_") + deprecated_env_value = os.getenv(deprecated_env, None) + if deprecated_env_value is not None: + if env_value is None: + env_value = deprecated_env_value + reason = f"{deprecated_env} is set. Use the REFLEX_ prefix for environment variables." + else: + reason = f"Both {self.name} and {deprecated_env} (deprecated) are set. Please use {self.name} only. Using {self.name}." + console.deprecate( + "Environment variables without REFLEX_ prefix are deprecated.", + reason=reason, + deprecation_version="0.0.0", + removal_version="0.0.0", + ) if env_value is not None: return self.interpret(env_value) return None @@ -392,17 +410,22 @@ class env_var: # type: ignore name: str default: Any - internal: bool = False + internal: bool + noprefix: bool - def __init__(self, default: Any, internal: bool = False) -> None: + def __init__( + self, default: Any, internal: bool = False, noprefix: bool = False + ) -> None: """Initialize the descriptor. Args: default: The default value. internal: Whether the environment variable is reflex internal. + noprefix: Whether to allow non-prefixed environment variables (deprecated). """ self.default = default self.internal = internal + self.noprefix = noprefix def __set_name__(self, owner, name): """Set the name of the descriptor. @@ -427,17 +450,20 @@ class env_var: # type: ignore env_name = self.name if self.internal: env_name = f"__{env_name}" - return EnvVar(name=env_name, default=self.default, type_=type_) + return EnvVar( + name=env_name, default=self.default, type_=type_, noprefix=self.noprefix + ) if TYPE_CHECKING: - def env_var(default, internal=False) -> EnvVar: + def env_var(default, internal: bool = False, noprefix: bool = False) -> EnvVar: """Typing helper for the env_var descriptor. Args: default: The default value. internal: Whether the environment variable is reflex internal. + noprefix: Whether to allow non-prefixed environment variables (deprecated). Returns: The EnvVar instance. @@ -459,16 +485,16 @@ class EnvironmentVariables: REFLEX_USE_NPM: EnvVar[bool] = env_var(False) # The npm registry to use. - NPM_CONFIG_REGISTRY: EnvVar[Optional[str]] = env_var(None) + REFLEX_NPM_CONFIG_REGISTRY: EnvVar[Optional[str]] = env_var(None, noprefix=True) # Whether to use Granian for the backend. Otherwise, use Uvicorn. REFLEX_USE_GRANIAN: EnvVar[bool] = env_var(False) # The username to use for authentication on python package repository. Username and password must both be provided. - TWINE_USERNAME: EnvVar[Optional[str]] = env_var(None) + REFLEX_TWINE_USERNAME: EnvVar[Optional[str]] = env_var(None, noprefix=True) # The password to use for authentication on python package repository. Username and password must both be provided. - TWINE_PASSWORD: EnvVar[Optional[str]] = env_var(None) + REFLEX_TWINE_PASSWORD: EnvVar[Optional[str]] = env_var(None, noprefix=True) # Whether to use the system installed bun. If set to false, bun will be bundled with the app. REFLEX_USE_SYSTEM_BUN: EnvVar[bool] = env_var(False) @@ -480,10 +506,12 @@ class EnvironmentVariables: REFLEX_WEB_WORKDIR: EnvVar[Path] = env_var(Path(constants.Dirs.WEB)) # Path to the alembic config file - ALEMBIC_CONFIG: EnvVar[ExistingPath] = env_var(Path(constants.ALEMBIC_CONFIG)) + REFLEX_ALEMBIC_CONFIG: EnvVar[ExistingPath] = env_var( + Path(constants.ALEMBIC_CONFIG), noprefix=True + ) # Disable SSL verification for HTTPX requests. - SSL_NO_VERIFY: EnvVar[bool] = env_var(False) + REFLEX_SSL_NO_VERIFY: EnvVar[bool] = env_var(False, noprefix=True) # The directory to store uploaded files. REFLEX_UPLOADED_FILES_DIR: EnvVar[Path] = env_var( @@ -500,7 +528,7 @@ class EnvironmentVariables: REFLEX_DIR: EnvVar[Path] = env_var(Path(constants.Reflex.DIR)) # Whether to print the SQL queries if the log level is INFO or lower. - SQLALCHEMY_ECHO: EnvVar[bool] = env_var(False) + REFLEX_SQLALCHEMY_ECHO: EnvVar[bool] = env_var(False, noprefix=True) # Whether to ignore the redis config error. Some redis servers only allow out-of-band configuration. REFLEX_IGNORE_REDIS_CONFIG_ERROR: EnvVar[bool] = env_var(False) @@ -528,22 +556,22 @@ class EnvironmentVariables: REFLEX_FRONTEND_ONLY: EnvVar[bool] = env_var(False) # Reflex internal env to reload the config. - RELOAD_CONFIG: EnvVar[bool] = env_var(False, internal=True) + REFLEX_RELOAD_CONFIG: EnvVar[bool] = env_var(False, internal=True) # If this env var is set to "yes", App.compile will be a no-op REFLEX_SKIP_COMPILE: EnvVar[bool] = env_var(False, internal=True) # Whether to run app harness tests in headless mode. - APP_HARNESS_HEADLESS: EnvVar[bool] = env_var(False) + REFLEX_APP_HARNESS_HEADLESS: EnvVar[bool] = env_var(False, noprefix=True) # Which app harness driver to use. - APP_HARNESS_DRIVER: EnvVar[str] = env_var("Chrome") + REFLEX_APP_HARNESS_DRIVER: EnvVar[str] = env_var("Chrome", noprefix=True) # Arguments to pass to the app harness driver. - APP_HARNESS_DRIVER_ARGS: EnvVar[str] = env_var("") + REFLEX_APP_HARNESS_DRIVER_ARGS: EnvVar[str] = env_var("", noprefix=True) # Where to save screenshots when tests fail. - SCREENSHOT_DIR: EnvVar[Optional[Path]] = env_var(None) + REFLEX_SCREENSHOT_DIR: EnvVar[Optional[Path]] = env_var(None, noprefix=True) environment = EnvironmentVariables() diff --git a/reflex/custom_components/custom_components.py b/reflex/custom_components/custom_components.py index 6be64ae2d..023bd3dd3 100644 --- a/reflex/custom_components/custom_components.py +++ b/reflex/custom_components/custom_components.py @@ -609,14 +609,14 @@ def publish( help="The API token to use for authentication on python package repository. If token is provided, no username/password should be provided at the same time", ), username: Optional[str] = typer.Option( - environment.TWINE_USERNAME.get(), + environment.REFLEX_TWINE_USERNAME.get(), "-u", "--username", show_default="TWINE_USERNAME environment variable value if set", help="The username to use for authentication on python package repository. Username and password must both be provided.", ), password: Optional[str] = typer.Option( - environment.TWINE_PASSWORD.get(), + environment.REFLEX_TWINE_PASSWORD.get(), "-p", "--password", show_default="TWINE_PASSWORD environment variable value if set", diff --git a/reflex/model.py b/reflex/model.py index 4b070ec67..6abb94d6b 100644 --- a/reflex/model.py +++ b/reflex/model.py @@ -38,12 +38,12 @@ def get_engine(url: str | None = None) -> sqlalchemy.engine.Engine: url = url or conf.db_url if url is None: raise ValueError("No database url configured") - if not environment.ALEMBIC_CONFIG.get().exists(): + if not environment.REFLEX_ALEMBIC_CONFIG.get().exists(): console.warn( "Database is not initialized, run [bold]reflex db init[/bold] first." ) # Print the SQL queries if the log level is INFO or lower. - echo_db_query = environment.SQLALCHEMY_ECHO.get() + echo_db_query = environment.REFLEX_SQLALCHEMY_ECHO.get() # Needed for the admin dash on sqlite. connect_args = {"check_same_thread": False} if url.startswith("sqlite") else {} return sqlmodel.create_engine(url, echo=echo_db_query, connect_args=connect_args) @@ -231,7 +231,7 @@ class Model(Base, sqlmodel.SQLModel): # pyright: ignore [reportGeneralTypeIssue Returns: tuple of (config, script_directory) """ - config = alembic.config.Config(environment.ALEMBIC_CONFIG.get()) + config = alembic.config.Config(environment.REFLEX_ALEMBIC_CONFIG.get()) return config, alembic.script.ScriptDirectory( config.get_main_option("script_location", default="version"), ) @@ -266,8 +266,8 @@ class Model(Base, sqlmodel.SQLModel): # pyright: ignore [reportGeneralTypeIssue def alembic_init(cls): """Initialize alembic for the project.""" alembic.command.init( - config=alembic.config.Config(environment.ALEMBIC_CONFIG.get()), - directory=str(environment.ALEMBIC_CONFIG.get().parent / "alembic"), + config=alembic.config.Config(environment.REFLEX_ALEMBIC_CONFIG.get()), + directory=str(environment.REFLEX_ALEMBIC_CONFIG.get().parent / "alembic"), ) @classmethod @@ -287,7 +287,7 @@ class Model(Base, sqlmodel.SQLModel): # pyright: ignore [reportGeneralTypeIssue Returns: True when changes have been detected. """ - if not environment.ALEMBIC_CONFIG.get().exists(): + if not environment.REFLEX_ALEMBIC_CONFIG.get().exists(): return False config, script_directory = cls._alembic_config() @@ -388,7 +388,7 @@ class Model(Base, sqlmodel.SQLModel): # pyright: ignore [reportGeneralTypeIssue True - indicating the process was successful. None - indicating the process was skipped. """ - if not environment.ALEMBIC_CONFIG.get().exists(): + if not environment.REFLEX_ALEMBIC_CONFIG.get().exists(): return with cls.get_db_engine().connect() as connection: diff --git a/reflex/reflex.py b/reflex/reflex.py index fa36b8601..947a51419 100644 --- a/reflex/reflex.py +++ b/reflex/reflex.py @@ -445,7 +445,7 @@ def db_init(): return # Check the alembic config. - if environment.ALEMBIC_CONFIG.get().exists(): + if environment.REFLEX_ALEMBIC_CONFIG.get().exists(): console.error( "Database is already initialized. Use " "[bold]reflex db makemigrations[/bold] to create schema change " diff --git a/reflex/testing.py b/reflex/testing.py index bb7ead2d9..52befab0f 100644 --- a/reflex/testing.py +++ b/reflex/testing.py @@ -617,10 +617,10 @@ class AppHarness: if self.frontend_url is None: raise RuntimeError("Frontend is not running.") want_headless = False - if environment.APP_HARNESS_HEADLESS.get(): + if environment.REFLEX_APP_HARNESS_HEADLESS.get(): want_headless = True if driver_clz is None: - requested_driver = environment.APP_HARNESS_DRIVER.get() + requested_driver = environment.REFLEX_APP_HARNESS_DRIVER.get() driver_clz = getattr(webdriver, requested_driver) if driver_options is None: driver_options = getattr(webdriver, f"{requested_driver}Options")() @@ -642,7 +642,7 @@ class AppHarness: driver_options.add_argument("headless") if driver_options is None: raise RuntimeError(f"Could not determine options for {driver_clz}") - if args := environment.APP_HARNESS_DRIVER_ARGS.get(): + if args := environment.REFLEX_APP_HARNESS_DRIVER_ARGS.get(): for arg in args.split(","): driver_options.add_argument(arg) if driver_option_args is not None: diff --git a/reflex/utils/net.py b/reflex/utils/net.py index acc202912..7bb803264 100644 --- a/reflex/utils/net.py +++ b/reflex/utils/net.py @@ -12,7 +12,7 @@ def _httpx_verify_kwarg() -> bool: Returns: True if SSL verification is enabled, False otherwise """ - return not environment.SSL_NO_VERIFY.get() + return not environment.REFLEX_SSL_NO_VERIFY.get() def get(url: str, **kwargs) -> httpx.Response: diff --git a/reflex/utils/prerequisites.py b/reflex/utils/prerequisites.py index a712e9a38..0c90073ed 100644 --- a/reflex/utils/prerequisites.py +++ b/reflex/utils/prerequisites.py @@ -281,7 +281,7 @@ def get_app(reload: bool = False) -> ModuleType: from reflex.utils import telemetry try: - environment.RELOAD_CONFIG.set(reload) + environment.REFLEX_RELOAD_CONFIG.set(reload) config = get_config() if not config.app_name: raise RuntimeError( @@ -1179,7 +1179,7 @@ def check_db_initialized() -> bool: """ if ( get_config().db_url is not None - and not environment.ALEMBIC_CONFIG.get().exists() + and not environment.REFLEX_ALEMBIC_CONFIG.get().exists() ): console.error( "Database is not initialized. Run [bold]reflex db init[/bold] first." @@ -1190,7 +1190,10 @@ def check_db_initialized() -> bool: def check_schema_up_to_date(): """Check if the sqlmodel metadata matches the current database schema.""" - if get_config().db_url is None or not environment.ALEMBIC_CONFIG.get().exists(): + if ( + get_config().db_url is None + or not environment.REFLEX_ALEMBIC_CONFIG.get().exists() + ): return with model.Model.get_db_engine().connect() as connection: try: diff --git a/reflex/utils/registry.py b/reflex/utils/registry.py index d98178c61..83704fb6c 100644 --- a/reflex/utils/registry.py +++ b/reflex/utils/registry.py @@ -55,4 +55,4 @@ def _get_npm_registry() -> str: Returns: str: """ - return environment.NPM_CONFIG_REGISTRY.get() or get_best_registry() + return environment.REFLEX_NPM_CONFIG_REGISTRY.get() or get_best_registry() diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index f7b825f16..52d2b9ab6 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -22,7 +22,10 @@ def xvfb(): Yields: the pyvirtualdisplay object that the browser will be open on """ - if os.environ.get("GITHUB_ACTIONS") and not environment.APP_HARNESS_HEADLESS.get(): + if ( + os.environ.get("GITHUB_ACTIONS") + and not environment.REFLEX_APP_HARNESS_HEADLESS.get() + ): from pyvirtualdisplay.smartdisplay import ( # pyright: ignore [reportMissingImports] SmartDisplay, ) @@ -43,7 +46,7 @@ def pytest_exception_interact(node, call, report): call: The pytest call describing when/where the test was invoked. report: The pytest log report object. """ - screenshot_dir = environment.SCREENSHOT_DIR.get() + screenshot_dir = environment.REFLEX_SCREENSHOT_DIR.get() if DISPLAY is None or screenshot_dir is None: return diff --git a/tests/units/test_config.py b/tests/units/test_config.py index e5d4622bd..71e64a54f 100644 --- a/tests/units/test_config.py +++ b/tests/units/test_config.py @@ -8,6 +8,7 @@ import pytest import reflex as rx import reflex.config from reflex.config import ( + EnvironmentVariables, EnvVar, env_var, environment, @@ -280,3 +281,13 @@ def test_env_var(): assert TestEnv.BOOLEAN.get() is False TestEnv.BOOLEAN.set(None) assert "BOOLEAN" not in os.environ + + +def test_deprecated_env_prefix() -> None: + deprecated_env = "ALEMBIC_CONFIG" + os.environ[deprecated_env] = "alembic.ini" + assert EnvironmentVariables.REFLEX_ALEMBIC_CONFIG.get() == Path("alembic.ini") + + # New env vars takes precedence + EnvironmentVariables.REFLEX_ALEMBIC_CONFIG.set(Path("alembic2.ini")) + assert EnvironmentVariables.REFLEX_ALEMBIC_CONFIG.get() == Path("alembic2.ini")