Skip to content

Add V2 UI packaging and Docker integration#8

Open
tomerqodo wants to merge 2 commits into
claude_claude_vs_qodo_base_add_v2_ui_packaging_and_docker_integration_pr8from
claude_claude_vs_qodo_head_add_v2_ui_packaging_and_docker_integration_pr8
Open

Add V2 UI packaging and Docker integration#8
tomerqodo wants to merge 2 commits into
claude_claude_vs_qodo_base_add_v2_ui_packaging_and_docker_integration_pr8from
claude_claude_vs_qodo_head_add_v2_ui_packaging_and_docker_integration_pr8

Conversation

@tomerqodo
Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#8

Comment thread Dockerfile
Comment on lines +75 to +82
# Install dependencies separately so they cache
COPY ./ui-v2/package*.json ./
RUN npm ci

# Build static UI files
COPY ./ui-v2 .
RUN npm run build

Copy link
Copy Markdown

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/api into the JS bundle at build time (via ui-v2/.env), and neither the Dockerfile nor CI overrides it. Unlike V1, which fetches /ui-settings at runtime for dynamic API URL resolution, V2 has no such mechanism — so all API calls from the browser will target localhost:4200 regardless of where the server is actually deployed. Since v2_enabled defaults to True, 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_URL to set its API base URL (see ui-v2/src/api/service.ts:4: const BASE_URL = import.meta.env.VITE_API_URL). Vite statically replaces import.meta.env.* references at build time, meaning whatever value is in the .env file gets permanently embedded in the JavaScript bundle.

The file ui-v2/.env sets VITE_API_URL=http://localhost:4200/api — an absolute URL pointing at localhost. There is no .env.production override, and neither the Dockerfile (RUN npm run build at line 81) nor the CI workflow (npm run build at line 49) set VITE_API_URL to 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 reach http://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

  1. ui-v2/.env contains VITE_API_URL=http://localhost:4200/api.
  2. During npm run build, Vite reads this .env file and replaces every occurrence of import.meta.env.VITE_API_URL in the source with the literal string "http://localhost:4200/api".
  3. The built JS bundle (in dist/) now contains hardcoded references to http://localhost:4200/api.
  4. The Dockerfile copies this bundle into the image (COPY --from=ui-v2-builder /opt/ui-v2/dist ./src/prefect/server/ui-v2) and the server serves it as static files.
  5. When a user opens the UI in their browser at https://my-prefect-server.example.com, the JS tries to call http://localhost:4200/api — which resolves to the user's own machine, not the server.
  6. All API requests fail with network errors.

Why existing code doesn't prevent this

The V1 UI solves this problem via a runtime /ui-settings endpoint (server.py lines 482-491) that returns the dynamically-configured api_url from the PREFECT_UI_API_URL setting. 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-settings or ui_settings in ui-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_PLACEHOLDER in replace_placeholder_string_in_files), there is no equivalent placeholder for the API URL.

Impact

Since v2_enabled defaults to True in 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-settings fetch, where the V2 UI requests its API URL from the server at startup. Or (b) Use a build-time placeholder (similar to PREFECT_UI_SERVE_BASE_REPLACE_PLACEHOLDER) that gets replaced post-build by replace_placeholder_string_in_files with 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.

Comment on lines +462 to +469
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}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Copy-paste bug: line 464 sets static_subpath = prefect.__ui_static_subpath__ in the v2_enabled=True branch, identical to the V1 branch. It should be prefect.__ui_v2_static_subpath__ (which was defined in __init__.py line 69 specifically for this purpose but is never referenced). This causes V2 UI files to be copied into the V1 build directory (server/ui_build), overwriting V1 files and creating conflicts if two processes use different v2_enabled settings.

Extended reasoning...

What the bug is

In create_ui_app(), the code selects which UI to serve based on the v2_enabled setting. While source_static_path is correctly set to prefect.__ui_v2_static_path__ for V2, the static_subpath variable (which determines the build/output directory) is set to prefect.__ui_static_subpath__ (server/ui_build) in both branches:

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 code

In src/prefect/__init__.py line 69, __ui_v2_static_subpath__ is defined as server/ui_v2_build — clearly intended to give V2 its own isolated build directory. However, this variable is never referenced anywhere in the codebase, making it dead code.

Step-by-step proof of the bug

  1. User sets v2_enabled=True (the default per this PR).
  2. create_ui_app() runs, entering the if v2_enabled branch.
  3. source_static_path is set to prefect.__ui_v2_static_path__ (server/ui-v2) — correct.
  4. static_subpath is set to prefect.__ui_static_subpath__ (server/ui_build) — wrong, should be server/ui_v2_build.
  5. static_dir resolves to server/ui_build (assuming no override).
  6. copy_directory() copies V2 source files from server/ui-v2 into server/ui_build, overwriting any V1 files that were there.
  7. If a second Prefect process on the same machine has v2_enabled=False, it will also target server/ui_build, triggering a re-copy of V1 files and overwriting the V2 files — the two processes fight over the same directory.

Impact

  • V2 UI files overwrite V1 files in the shared build directory, corrupting the V1 build.
  • Two Prefect processes with different v2_enabled settings will continuously overwrite each other's files since the cache_key differs (v1: vs v2: prefix) but the target directory is the same.
  • The __ui_v2_static_subpath__ variable defined in __init__.py is completely unused dead code.

Fix

Line 464 should be changed to:

static_subpath = prefect.__ui_v2_static_subpath__

Comment thread src/prefect/__init__.py
__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"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 __ui_v2_static_path__ is set to server/ui_v2 (underscore) but all build processes (Dockerfile, CI, pyproject.toml) output V2 UI files to server/ui-v2 (hyphen). Since v2_enabled defaults to True, os.path.exists(source_static_path) in create_ui_app will always return False, silently preventing the V2 UI from ever being served. Change "ui_v2" to "ui-v2" on this line (and the same for ui_v2_buildui-v2_build on line 69).

Extended reasoning...

Path mismatch between Python code and build artifacts

In src/prefect/__init__.py line 72, __ui_v2_static_path__ is defined as:

__ui_v2_static_path__: pathlib.Path = __module_path__ / "server" / "ui_v2"

This resolves to a filesystem path ending in server/ui_v2 (with an underscore).

However, every build process that produces V2 UI artifacts uses ui-v2 (with a hyphen):

  • Dockerfile line 106: COPY --from=ui-v2-builder /opt/ui-v2/dist ./src/prefect/server/ui-v2
  • GitHub Actions CI: cp -r dist ../src/prefect/server/ui-v2
  • pyproject.toml artifacts: "src/prefect/server/ui-v2"

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 ui-v2 (hyphen), not ui_v2 (underscore).

Step-by-step proof of failure

  1. User installs prefect (or builds Docker image). The V2 UI files land in <site-packages>/prefect/server/ui-v2/.
  2. User starts the server with prefect server start. The new v2_enabled setting defaults to True.
  3. create_ui_app() in server.py sets source_static_path = prefect.__ui_v2_static_path__, which resolves to .../prefect/server/ui_v2.
  4. At line 525, the guard os.path.exists(source_static_path) checks for .../prefect/server/ui_v2 — this directory does not exist (only ui-v2 exists).
  5. The check returns False, so the entire UI mount block is skipped. No static files are served, no error is logged.
  6. The server starts successfully but serves no UI at all.

Impact

This is the primary feature of the PR and it is completely broken out of the box. Since v2_enabled defaults to True, every user who upgrades will silently lose their UI — the V1 UI is also skipped because the code takes the V2 branch. There is no error message or warning to help diagnose the issue.

Fix

Change line 72 from "ui_v2" to "ui-v2" to match the build output directory. The same fix should be applied to __ui_v2_static_subpath__ on line 69 (change "ui_v2_build" to "ui-v2_build" for consistency, though this path is not currently used distinctly for V2).

Comment on lines +22 to +26
v2_enabled: bool = Field(
default=True,
description="Whether to serve the experimental V2 UI instead of the default V1 UI.",
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 description

The 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 UI

At cli/server.py:117, the CLI checks whether to show "Check out the dashboard" or "The dashboard is not built" by testing:

if not os.path.exists(prefect.__ui_static_path__):
    blurb += dashboard_not_built

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

  1. Developer runs prefect dev build-ui, which copies ui/dist to prefect.ui_static_path (the V1 path at server/ui) -- see cli/dev.py:107-112. There is no V2 equivalent.
  2. Developer runs prefect server start.
  3. The CLI checks ui_static_path (V1 path), finds it exists, and prints "Check out the dashboard at ...".
  4. But create_ui_app looks for ui_v2_static_path (server/ui_v2), which does not exist.
  5. The os.path.exists(source_static_path) check at server.py:522 fails, so no UI is mounted.
  6. The user sees a welcome message pointing them to a dashboard that returns 404.

Reverse scenario is also broken

If 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.

Impact

This 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

  1. Change v2_enabled default to False so V2 is opt-in, matching the "experimental" description.
  2. Update cli/server.py:117 to check the appropriate path based on v2_enabled (i.e., check ui_v2_static_path when V2 is enabled).
  3. Add a V2 build-ui command or extend prefect dev build-ui to support V2.

Comment on lines 528 to +529
):
# Log which UI version is being served
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 This uses logging.getLogger("ui_server") instead of the project-standard get_logger("ui_server") which is already imported and used elsewhere in this file (line 55, 77). This creates a logger outside the prefect.* namespace, so it won't inherit prefect's logging configuration or the ObfuscateApiKeyFilter.

Extended reasoning...

What the bug is

The new V2 UI logging code at line 528-529 uses logging.getLogger("ui_server") directly instead of the project-standard get_logger("ui_server"). The get_logger function is already imported at line 55 (from prefect.logging import get_logger) and used at line 77 (logger = get_logger("server")).

How get_logger differs from logging.getLogger

Looking at src/prefect/logging/loggers.py:64-88, get_logger("ui_server") does two important things that logging.getLogger("ui_server") does not:

  1. It creates the logger under the prefect namespace as prefect.ui_server (via parent_logger.getChild(name)), so it inherits the prefect logging hierarchy and respects PREFECT_SERVER_LOGGING_LEVEL.
  2. It adds the ObfuscateApiKeyFilter to prevent API keys from being logged in plain text.

Using logging.getLogger("ui_server") creates a standalone top-level logger named ui_server that is completely outside the prefect logging hierarchy.

Step-by-step proof

  1. User starts the prefect server with PREFECT_SERVER_LOGGING_LEVEL=WARNING and V2 UI enabled.
  2. create_ui_app() is called, and v2_enabled is True.
  3. Line 528 executes: ui_logger = logging.getLogger("ui_server") — this creates a logger named "ui_server" (not "prefect.ui_server").
  4. Line 529 executes: ui_logger.info("Serving experimental V2 UI") — since this logger is not under the prefect.* hierarchy, it uses Python's root logger configuration, not prefect's. The log level setting PREFECT_SERVER_LOGGING_LEVEL=WARNING has no effect on this logger.
  5. By contrast, if get_logger("ui_server") were used, the logger would be prefect.ui_server, inheriting the prefect parent logger's level and filters.

Impact

The practical impact is low since this is a single info-level log message. However, it violates the established pattern in the file and the project's AGENTS.md Rule 3, which mandates using get_logger() for all logger instances. The logger also misses the ObfuscateApiKeyFilter that get_logger adds.

Fix

Replace:

ui_logger = logging.getLogger("ui_server")

with:

ui_logger = get_logger("ui_server")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant