Add V2 UI packaging and Docker integration#137
Conversation
Code Review by Qodo
1. ui_logger uses logging.getLogger()
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+531
to
+532
| ui_logger = logging.getLogger("ui_server") | ||
| ui_logger.info("Serving experimental V2 UI") |
There was a problem hiding this comment.
1. ui_logger uses logging.getlogger() 📘 Rule violation ✧ Quality
• The new code instantiates ui_logger via logging.getLogger("ui_server") without the
standardized get_logger(...) pattern and without a logger type annotation.
• This breaks consistency in logging configuration/type-safety expectations and can lead to uneven
formatting/handlers compared to other loggers created via get_logger.
Agent prompt
## Issue description
A new logger is created with `logging.getLogger("ui_server")`, which violates the required standardized logger initialization pattern (`logger: "logging.Logger" = get_logger("...")`).
## Issue Context
This code path runs when serving the UI, so consistent logger configuration matters for downstream handlers/formatters and type safety.
## 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 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" |
There was a problem hiding this comment.
2. prefect.init missing future annotations 📘 Rule violation ⛯ Reliability
• src/prefect/__init__.py contains type annotations (including newly added annotated constants) but does not include from __future__ import annotations as the first import statement. • This violates the required typing standard and can reduce forward-reference robustness and type-checking performance.
Agent prompt
## Issue description
`src/prefect/__init__.py` uses type annotations but is missing `from __future__ import annotations` as the first import statement.
## Issue Context
Compliance requires future-annotations in any `src/` file containing type hints to improve forward-reference behavior and type-checking performance.
## Fix Focus Areas
- src/prefect/__init__.py[1-10]
- src/prefect/__init__.py[55-74]
ⓘ 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.", | ||
| ) |
There was a problem hiding this comment.
3. serveruisettings missing future annotations 📘 Rule violation ⛯ Reliability
• src/prefect/settings/models/server/ui.py contains type annotations (including the newly added v2_enabled: bool) but does not include from __future__ import annotations as the first import statement. • This violates the typing standard and may cause avoidable runtime import coupling for annotations in more complex cases.
Agent prompt
## Issue description
`src/prefect/settings/models/server/ui.py` contains type annotations but lacks the required `from __future__ import annotations` import as the first import statement.
## Issue Context
Compliance requires future-annotations for all `src/` Python files that use type hints.
## Fix Focus Areas
- src/prefect/settings/models/server/ui.py[1-10]
- src/prefect/settings/models/server/ui.py[20-35]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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" |
There was a problem hiding this comment.
4. V2 ui path mismatch 🐞 Bug ✓ Correctness
• Prefect’s runtime constant for the V2 UI directory points to server/ui_v2, but CI/Docker packaging writes the built assets to server/ui-v2. • When v2_enabled is true, the UI server checks os.path.exists(source_static_path) and will skip mounting the UI if the path does not exist. • This will likely ship an install where the V2 UI is never served (and since v2_enabled defaults true, it can effectively disable the UI by default).
Agent prompt
### Issue description
Runtime V2 UI path constants (`prefect.__ui_v2_static_path__`) point to `server/ui_v2` (underscore), but Docker/CI/Hatch package the V2 UI into `server/ui-v2` (hyphen). When V2 is enabled, the server checks `os.path.exists(source_static_path)` and will not mount the UI if the directory is missing.
### Issue Context
This can cause Prefect Server to ship with `v2_enabled=True` but no UI being served, because the server is looking in a different directory than the one containing the packaged artifacts.
### Fix Focus Areas
- src/prefect/__init__.py[67-72]
- Dockerfile[102-107]
- .github/workflows/python-package.yaml[46-52]
- pyproject.toml[231-233]
- src/prefect/server/api/server.py[462-466]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+462
to
+465
| 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}" |
There was a problem hiding this comment.
5. Wrong v2 cache subpath 🐞 Bug ⛯ Reliability
• When v2_enabled is true, the code sets static_subpath to the V1 __ui_static_subpath__ instead of the V2 __ui_v2_static_subpath__. • This means V2 assets would be copied into the V1 build/cache directory, potentially overwriting V1 assets and causing confusing behavior when toggling V1/V2. • The dedicated V2 subpath constant is currently unused, indicating this is likely an oversight.
Agent prompt
### Issue description
In `create_ui_app`, the V2 code path sets `static_subpath` to the V1 value (`prefect.__ui_static_subpath__`). As a result, V2 UI assets would be copied into the V1 UI cache/build directory, potentially overwriting V1 assets.
### Issue Context
`src/prefect/__init__.py` defines `__ui_v2_static_subpath__` explicitly, suggesting the design intends separate cache/build directories.
### Fix Focus Areas
- src/prefect/server/api/server.py[462-474]
- src/prefect/__init__.py[60-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.", | ||
| ) |
There was a problem hiding this comment.
6. Experimental enabled by default 🐞 Bug ⛯ Reliability
• The new server.ui.v2_enabled flag defaults to True while explicitly described as “experimental”. • If V2 assets are missing/mismatched (as in the path issues above), this default can cause the server to start without mounting any UI. • Even if assets are present, enabling an experimental UI by default may be an unintended breaking UX change for existing users.
Agent prompt
### Issue description
`server.ui.v2_enabled` is described as experimental but defaults to `True`, which can unintentionally switch users to the experimental UI (or result in no UI being served if assets are missing).
### Issue Context
There is already logic that only mounts the UI if `os.path.exists(source_static_path)`. With the current path mismatch, the default `True` setting can disable UI serving entirely.
### Fix Focus Areas
- src/prefect/settings/models/server/ui.py[22-25]
- src/prefect/server/api/server.py[459-469]
- src/prefect/server/api/server.py[524-528]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Benchmark PR from qodo-benchmark#541