-
Notifications
You must be signed in to change notification settings - Fork 0
Add V2 UI packaging and Docker integration #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: claude_claude_vs_qodo_base_add_v2_ui_packaging_and_docker_integration_pr8
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,6 +64,13 @@ class VersionInfo(TypedDict("_FullRevisionId", {"full-revisionid": str})): | |
| # The absolute path to the built UI within the Python module | ||
| __ui_static_path__: pathlib.Path = __module_path__ / "server" / "ui" | ||
|
|
||
| # The absolute path to the built V2 UI within the Python module, used by | ||
| # `prefect server start` to serve a dynamic build of the V2 UI | ||
| __ui_v2_static_subpath__: pathlib.Path = __module_path__ / "server" / "ui_v2_build" | ||
|
|
||
| # The absolute path to the built V2 UI within the Python module | ||
| __ui_v2_static_path__: pathlib.Path = __module_path__ / "server" / "ui_v2" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Extended reasoning...Path mismatch between Python code and build artifactsIn __ui_v2_static_path__: pathlib.Path = __module_path__ / "server" / "ui_v2"This resolves to a filesystem path ending in However, every build process that produces V2 UI artifacts uses
These are plain filesystem directory names — Python/pip packaging normalizes top-level distribution names (PEP 503) but does not rename subdirectories within a package. So the installed directory will be Step-by-step proof of failure
ImpactThis is the primary feature of the PR and it is completely broken out of the box. Since FixChange line 72 from |
||
|
|
||
| del _build_info, pathlib | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -455,11 +455,22 @@ async def token_validation(request: Request, call_next: Any): # type: ignore[re | |
| def create_ui_app(ephemeral: bool) -> FastAPI: | ||
| ui_app = FastAPI(title=UI_TITLE) | ||
| base_url = prefect.settings.PREFECT_UI_SERVE_BASE.value() | ||
| cache_key = f"{prefect.__version__}:{base_url}" | ||
|
|
||
| # Determine which UI to serve based on setting | ||
| v2_enabled = prefect.settings.get_current_settings().server.ui.v2_enabled | ||
|
|
||
| if v2_enabled: | ||
| source_static_path = prefect.__ui_v2_static_path__ | ||
| static_subpath = prefect.__ui_static_subpath__ | ||
| cache_key = f"v2:{prefect.__version__}:{base_url}" | ||
| else: | ||
| source_static_path = prefect.__ui_static_path__ | ||
| static_subpath = prefect.__ui_static_subpath__ | ||
| cache_key = f"v1:{prefect.__version__}:{base_url}" | ||
|
Comment on lines
+462
to
+469
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Copy-paste bug: line 464 sets Extended reasoning...What the bug isIn if v2_enabled:
source_static_path = prefect.__ui_v2_static_path__
static_subpath = prefect.__ui_static_subpath__ # BUG
cache_key = f"v2:{prefect.__version__}:{base_url}"
else:
source_static_path = prefect.__ui_static_path__
static_subpath = prefect.__ui_static_subpath__
cache_key = f"v1:{prefect.__version__}:{base_url}"The dedicated V2 subpath is dead codeIn Step-by-step proof of the bug
Impact
FixLine 464 should be changed to: static_subpath = prefect.__ui_v2_static_subpath__ |
||
|
|
||
| stripped_base_url = base_url.rstrip("/") | ||
| static_dir = ( | ||
| prefect.settings.PREFECT_UI_STATIC_DIRECTORY.value() | ||
| or prefect.__ui_static_subpath__ | ||
| static_dir = prefect.settings.PREFECT_UI_STATIC_DIRECTORY.value() or str( | ||
| static_subpath | ||
| ) | ||
| reference_file_name = "UI_SERVE_BASE" | ||
|
|
||
|
|
@@ -495,7 +506,7 @@ def create_ui_static_subpath() -> None: | |
| if not os.path.exists(static_dir): | ||
| os.makedirs(static_dir) | ||
|
|
||
| copy_directory(str(prefect.__ui_static_path__), str(static_dir)) | ||
| copy_directory(str(source_static_path), str(static_dir)) | ||
| replace_placeholder_string_in_files( | ||
| str(static_dir), | ||
| "/PREFECT_UI_SERVE_BASE_REPLACE_PLACEHOLDER", | ||
|
|
@@ -511,10 +522,15 @@ def create_ui_static_subpath() -> None: | |
| ui_app.add_middleware(GZipMiddleware) | ||
|
|
||
| if ( | ||
| os.path.exists(prefect.__ui_static_path__) | ||
| os.path.exists(source_static_path) | ||
| and prefect.settings.PREFECT_UI_ENABLED.value() | ||
| and not ephemeral | ||
| ): | ||
| # Log which UI version is being served | ||
|
Comment on lines
528
to
+529
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 This uses Extended reasoning...What the bug isThe new V2 UI logging code at line 528-529 uses How
|
||
| if v2_enabled: | ||
| ui_logger = logging.getLogger("ui_server") | ||
| ui_logger.info("Serving experimental V2 UI") | ||
|
|
||
| # If the static files have already been copied, check if the base_url has changed | ||
| # If it has, we delete the subpath directory and copy the files again | ||
| if not reference_file_matches_base_url(): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,11 @@ class ServerUISettings(PrefectBaseSettings): | |
| ), | ||
| ) | ||
|
|
||
| v2_enabled: bool = Field( | ||
| default=True, | ||
| description="Whether to serve the experimental V2 UI instead of the default V1 UI.", | ||
| ) | ||
|
|
||
|
Comment on lines
+22
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 The v2_enabled setting defaults to True, but its description calls V2 "experimental" and V1 "the default" -- this is self-contradictory and silently switches all existing users to V2 on upgrade. Additionally, the CLI startup check at cli/server.py:117 still checks prefect.ui_static_path (V1 path) and prefect dev build-ui only builds V1, so with V2 as the default the startup message will be misleading (says "dashboard available" when V2 files may not exist). Consider defaulting v2_enabled to False and updating the CLI check to be V2-aware. Extended reasoning...The default value contradicts the descriptionThe new v2_enabled field in ServerUISettings (settings/models/server/ui.py:22-26) has default=True but its description reads: "Whether to serve the experimental V2 UI instead of the default V1 UI." This is self-contradictory -- if V1 is called the "default" and V2 is called "experimental", then the actual default value should be False (opt-in). With default=True, every existing Prefect deployment will silently switch to the experimental V2 UI on upgrade with no opt-in. CLI startup message is now inconsistent with the served UIAt cli/server.py:117, the CLI checks whether to show "Check out the dashboard" or "The dashboard is not built" by testing: This always checks the V1 path (server/ui). But with v2_enabled=True, create_ui_app in server/api/server.py:462-463 sets source_static_path = prefect.ui_v2_static_path (i.e., server/ui_v2). The CLI check and the actual served path are now out of sync. Concrete scenario: developer workflow
Reverse scenario is also brokenIf only V2 files are present (e.g., in a Docker build that builds both, but someone manually removes V1), the CLI would say "The dashboard is not built" even though the server would correctly serve V2. The CLI message is always wrong in one direction or the other when the two UI versions have different build states. ImpactThis is a breaking change for all existing users on upgrade: they silently get an experimental UI (or worse, no UI at all if V2 static files are not bundled in their installation method). The Docker build and CI workflow do build both UIs, but pip-installed or development installations will not have V2 files, resulting in a completely broken dashboard with a misleading success message. Recommended fix
|
||
| api_url: Optional[str] = Field( | ||
| default=None, | ||
| description="The connection url for communication from the UI to the API. Defaults to `PREFECT_API_URL` if set. Otherwise, the default URL is generated from `PREFECT_SERVER_API_HOST` and `PREFECT_SERVER_API_PORT`.", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 The V2 UI bakes
VITE_API_URL=http://localhost:4200/apiinto the JS bundle at build time (viaui-v2/.env), and neither the Dockerfile nor CI overrides it. Unlike V1, which fetches/ui-settingsat runtime for dynamic API URL resolution, V2 has no such mechanism — so all API calls from the browser will targetlocalhost:4200regardless of where the server is actually deployed. Sincev2_enableddefaults toTrue, this makes the default UI non-functional in any Docker, cloud, or non-localhost deployment.Extended reasoning...
What the bug is
The V2 UI uses Vite's
import.meta.env.VITE_API_URLto set its API base URL (seeui-v2/src/api/service.ts:4:const BASE_URL = import.meta.env.VITE_API_URL). Vite statically replacesimport.meta.env.*references at build time, meaning whatever value is in the.envfile gets permanently embedded in the JavaScript bundle.The file
ui-v2/.envsetsVITE_API_URL=http://localhost:4200/api— an absolute URL pointing at localhost. There is no.env.productionoverride, and neither the Dockerfile (RUN npm run buildat line 81) nor the CI workflow (npm run buildat line 49) setVITE_API_URLto anything else.How it manifests
When a user deploys the Prefect server to any environment other than
localhost:4200(e.g., a Docker container exposed on a different port, a cloud VM, Kubernetes), the V2 UI JavaScript running in the user's browser will still attempt to reachhttp://localhost:4200/api. Since that address doesn't point to the actual Prefect server, every single API call from the UI will fail — the UI will be completely non-functional.Step-by-step proof
ui-v2/.envcontainsVITE_API_URL=http://localhost:4200/api.npm run build, Vite reads this.envfile and replaces every occurrence ofimport.meta.env.VITE_API_URLin the source with the literal string"http://localhost:4200/api".dist/) now contains hardcoded references tohttp://localhost:4200/api.COPY --from=ui-v2-builder /opt/ui-v2/dist ./src/prefect/server/ui-v2) and the server serves it as static files.https://my-prefect-server.example.com, the JS tries to callhttp://localhost:4200/api— which resolves to the user's own machine, not the server.Why existing code doesn't prevent this
The V1 UI solves this problem via a runtime
/ui-settingsendpoint (server.py lines 482-491) that returns the dynamically-configuredapi_urlfrom thePREFECT_UI_API_URLsetting. The V1 JS fetches this endpoint on startup and uses the returned URL for all API calls.The V2 UI has no equivalent mechanism — grep for
ui-settingsorui_settingsinui-v2/src/returns zero matches. Additionally, while the codebase has a post-build placeholder replacement mechanism for the base URL path (PREFECT_UI_SERVE_BASE_REPLACE_PLACEHOLDERinreplace_placeholder_string_in_files), there is no equivalent placeholder for the API URL.Impact
Since
v2_enableddefaults toTruein this PR, the V2 UI is the default. This means every non-localhost deployment will have a completely broken UI out of the box — users won't be able to view flows, runs, deployments, or any other data through the web interface.How to fix
Either: (a) Add a runtime configuration mechanism similar to V1's
/ui-settingsfetch, where the V2 UI requests its API URL from the server at startup. Or (b) Use a build-time placeholder (similar toPREFECT_UI_SERVE_BASE_REPLACE_PLACEHOLDER) that gets replaced post-build byreplace_placeholder_string_in_fileswith the correct API URL. Or (c) Use a relative URL (e.g.,/api) instead of an absolute localhost URL, which would work regardless of deployment host.