Skip to content

Add V2 UI packaging and Docker integration#162

Open
tomerqodo wants to merge 2 commits into
qodo_action_req_1_base_add_v2_ui_packaging_and_docker_integration_pr8from
qodo_action_req_1_head_add_v2_ui_packaging_and_docker_integration_pr8
Open

Add V2 UI packaging and Docker integration#162
tomerqodo wants to merge 2 commits into
qodo_action_req_1_base_add_v2_ui_packaging_and_docker_integration_pr8from
qodo_action_req_1_head_add_v2_ui_packaging_and_docker_integration_pr8

Conversation

@tomerqodo
Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#8

@qodo-code-review
Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (3) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Missing future import in __init__.py 📘 Rule violation ✓ Correctness
Description
src/prefect/__init__.py contains type annotations (e.g., __ui_v2_static_path__: pathlib.Path)
  but does not include from __future__ import annotations as the first import.
• This violates the requirement for src/ Python files with type hints to enable forward-reference
  resolution and consistent type-checking behavior.
Code

src/prefect/init.py[R67-72]

+# 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"
Evidence
The checklist requires from __future__ import annotations as the first import in any src/ Python
file that uses type annotations. src/prefect/__init__.py has annotated variables added/maintained,
but its import block begins with normal imports instead of the required future import.

AGENTS.md
src/prefect/init.py[1-10]
src/prefect/init.py[55-72]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`src/prefect/__init__.py` uses type annotations but does not have `from __future__ import annotations` as the first import, violating the project rule for typed files under `src/`.

## Issue Context
This file contains annotated constants (e.g., `__ui_v2_static_path__: pathlib.Path`) and imports from `typing`, so it must include the future annotations import as the first import statement.

## Fix Focus Areas
- src/prefect/__init__.py[1-10]
- src/prefect/__init__.py[55-72]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Missing future import in ui.py 📘 Rule violation ✓ Correctness
Description
src/prefect/settings/models/server/ui.py contains type annotations (e.g., `model_config:
  ClassVar[...], v2_enabled: bool) but lacks from __future__ import annotations` as the first
  import.
• This violates the required typing import convention for src/ modules and can lead to
  inconsistent annotation evaluation across the codebase.
Code

src/prefect/settings/models/server/ui.py[R22-25]

+    v2_enabled: bool = Field(
+        default=True,
+        description="Whether to serve the experimental V2 UI instead of the default V1 UI.",
+    )
Evidence
The compliance rule requires from __future__ import annotations to be present as the first import
in any typed Python file under src/. This file begins with from typing ... and defines annotated
settings fields, but has no future import at all.

AGENTS.md
src/prefect/settings/models/server/ui.py[1-7]
src/prefect/settings/models/server/ui.py[9-29]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`src/prefect/settings/models/server/ui.py` defines type annotations but does not include `from __future__ import annotations` as the first import.

## Issue Context
The file defines annotated fields and class variables; the project rule requires future annotations import first for all typed files under `src/`.

## Fix Focus Areas
- src/prefect/settings/models/server/ui.py[1-7]
- src/prefect/settings/models/server/ui.py[9-29]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. logging.getLogger() used for ui_logger 📘 Rule violation ✧ Quality
Description
• The new code creates ui_logger via logging.getLogger("ui_server") rather than the required
  standardized get_logger(...) pattern with a logging.Logger type annotation.
• This breaks logging consistency/type-safety expectations and violates the project’s logger
  instantiation standard.
Code

src/prefect/server/api/server.py[R529-532]

+        # Log which UI version is being served
+        if v2_enabled:
+            ui_logger = logging.getLogger("ui_server")
+            ui_logger.info("Serving experimental V2 UI")
Evidence
The compliance rule mandates using logger: "logging.Logger" = get_logger("module_name") instead of
logging.getLogger() directly. In create_ui_app, the new ui_logger is instantiated with
logging.getLogger without the standardized pattern or type annotation.

AGENTS.md
src/prefect/server/api/server.py[529-533]
src/prefect/server/api/server.py[51-52]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new logger is created via `logging.getLogger("ui_server")` inside `create_ui_app`, which violates the standardized logger initialization pattern that requires `get_logger(...)` and a `logging.Logger` type annotation.

## Issue Context
This module already imports `get_logger` and defines a module-level `logger: "logging.Logger" = get_logger("server")`.

## Fix Focus Areas
- src/prefect/server/api/server.py[529-533]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (1)
4. V2 UI path mismatch 🐞 Bug ✓ Correctness
Description
• Python runtime looks for V2 UI assets in a different directory name than the build/packaging steps
  produce (underscore vs hyphen).
• When V2 is enabled, os.path.exists(source_static_path) will fail and the server will not mount
  any UI routes, effectively disabling the UI.
• This is likely to reproduce in Docker builds and release packaging because both Dockerfile and
  hatch artifacts use the hyphenated directory name.
Code

src/prefect/init.py[R67-72]

+# 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"
Evidence
The PR introduces __ui_v2_static_path__ pointing to server/ui_v2, and server UI mounting checks
that path. However, the Docker build and hatch packaging place V2 assets under server/ui-v2
(hyphen). That mismatch means the V2 asset directory the server checks for will not exist in the
packaged artifact/image.

src/prefect/init.py[60-73]
src/prefect/server/api/server.py[455-469]
src/prefect/server/api/server.py[524-543]
Dockerfile[102-107]
pyproject.toml[231-233]
.github/workflows/python-package.yaml[46-52]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
V2 UI assets are built/copied into `src/prefect/server/ui-v2`, but the Python runtime expects them in `src/prefect/server/ui_v2`.

### Issue Context
When `server.ui.v2_enabled` is true, `create_ui_app` checks `os.path.exists(prefect.__ui_v2_static_path__)` and mounts the UI only if that directory exists. Because the packaged assets land in a different directory name, V2 UI serving will silently not mount.

### Fix Focus Areas
- src/prefect/__init__.py[67-72]
- Dockerfile[102-107]
- pyproject.toml[231-233]
- .github/workflows/python-package.yaml[46-52]
- src/prefect/server/api/server.py[459-469]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



ⓘ The new review experience is currently in Beta. Learn more

Qodo Logo

Comment thread src/prefect/__init__.py
Comment on lines +67 to +72
# 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"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Missing future import in init.py 📘 Rule violation ✓ Correctness

src/prefect/__init__.py contains type annotations (e.g., __ui_v2_static_path__: pathlib.Path)
  but does not include from __future__ import annotations as the first import.
• This violates the requirement for src/ Python files with type hints to enable forward-reference
  resolution and consistent type-checking behavior.
Agent prompt
## Issue description
`src/prefect/__init__.py` uses type annotations but does not have `from __future__ import annotations` as the first import, violating the project rule for typed files under `src/`.

## Issue Context
This file contains annotated constants (e.g., `__ui_v2_static_path__: pathlib.Path`) and imports from `typing`, so it must include the future annotations import as the first import statement.

## Fix Focus Areas
- src/prefect/__init__.py[1-10]
- src/prefect/__init__.py[55-72]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +22 to +25
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.

Action required

2. Missing future import in ui.py 📘 Rule violation ✓ Correctness

src/prefect/settings/models/server/ui.py contains type annotations (e.g., `model_config:
  ClassVar[...], v2_enabled: bool) but lacks from __future__ import annotations` as the first
  import.
• This violates the required typing import convention for src/ modules and can lead to
  inconsistent annotation evaluation across the codebase.
Agent prompt
## Issue description
`src/prefect/settings/models/server/ui.py` defines type annotations but does not include `from __future__ import annotations` as the first import.

## Issue Context
The file defines annotated fields and class variables; the project rule requires future annotations import first for all typed files under `src/`.

## Fix Focus Areas
- src/prefect/settings/models/server/ui.py[1-7]
- src/prefect/settings/models/server/ui.py[9-29]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +529 to +532
# Log which UI version is being served
if v2_enabled:
ui_logger = logging.getLogger("ui_server")
ui_logger.info("Serving experimental V2 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.

Action required

3. logging.getlogger() used for ui_logger 📘 Rule violation ✧ Quality

• The new code creates ui_logger via logging.getLogger("ui_server") rather than the required
  standardized get_logger(...) pattern with a logging.Logger type annotation.
• This breaks logging consistency/type-safety expectations and violates the project’s logger
  instantiation standard.
Agent prompt
## Issue description
A new logger is created via `logging.getLogger("ui_server")` inside `create_ui_app`, which violates the standardized logger initialization pattern that requires `get_logger(...)` and a `logging.Logger` type annotation.

## Issue Context
This module already imports `get_logger` and defines a module-level `logger: "logging.Logger" = get_logger("server")`.

## Fix Focus Areas
- src/prefect/server/api/server.py[529-533]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread src/prefect/__init__.py
Comment on lines +67 to +72
# 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"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

4. V2 ui path mismatch 🐞 Bug ✓ Correctness

• Python runtime looks for V2 UI assets in a different directory name than the build/packaging steps
  produce (underscore vs hyphen).
• When V2 is enabled, os.path.exists(source_static_path) will fail and the server will not mount
  any UI routes, effectively disabling the UI.
• This is likely to reproduce in Docker builds and release packaging because both Dockerfile and
  hatch artifacts use the hyphenated directory name.
Agent prompt
### Issue description
V2 UI assets are built/copied into `src/prefect/server/ui-v2`, but the Python runtime expects them in `src/prefect/server/ui_v2`.

### Issue Context
When `server.ui.v2_enabled` is true, `create_ui_app` checks `os.path.exists(prefect.__ui_v2_static_path__)` and mounts the UI only if that directory exists. Because the packaged assets land in a different directory name, V2 UI serving will silently not mount.

### Fix Focus Areas
- src/prefect/__init__.py[67-72]
- Dockerfile[102-107]
- pyproject.toml[231-233]
- .github/workflows/python-package.yaml[46-52]
- src/prefect/server/api/server.py[459-469]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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