Copilot commented on code in PR #63561:
URL: https://github.com/apache/airflow/pull/63561#discussion_r2935264214
##########
dev/breeze/src/airflow_breeze/commands/ui_commands.py:
##########
@@ -752,5 +752,5 @@ def compile_ui_assets(dev: bool, force_clean: bool):
dev=dev, run_in_background=False, force_clean=force_clean,
additional_ui_hooks=[]
)
if compile_ui_assets_result.returncode != 0:
- get_console().print("[warn]New assets were generated[/]")
+ console_print("[warn]New assets were generated[/]")
Review Comment:
`[warn]` is not a valid style tag in the Breeze Rich theme (the theme
defines `warning`). This may raise a Rich markup/style parsing error at
runtime. Use `[warning]...[/]` (or disable markup) instead.
##########
dev/breeze/src/airflow_breeze/utils/github.py:
##########
@@ -323,7 +319,7 @@ def download_artifact_from_pr(pr: str, output_file: Path,
github_repository: str
pull_response = session.get(pr_url, headers=headers)
if pull_response.status_code != 200:
- get_console().print(
+ console_print(
f"[error]Fetching PR failed with status codee
{pull_response.status_code}: {pull_response.text}",
)
sys.exit(1)
Review Comment:
Typo in error message: `status codee` should be `status code`.
##########
dev/breeze/src/airflow_breeze/utils/platforms.py:
##########
@@ -38,15 +38,15 @@ def _exists_no_permission_error(p: str) -> bool:
def message_on_wsl1_detected(release_name: str | None, kernel_version:
tuple[int, ...] | None):
- from airflow_breeze.utils.console import get_console
+ from airflow_breeze.utils.console import console_print
Review Comment:
Avoid importing `console_print` inside the function body; it can be imported
at module level (there’s no apparent circular dependency here). Keeping imports
at the top improves readability and follows the project guideline against
in-function imports.
##########
dev/breeze/src/airflow_breeze/commands/main_command.py:
##########
@@ -184,15 +182,13 @@ def check_for_rosetta_environment():
stderr=subprocess.DEVNULL,
).strip()
if runs_in_rosetta == "1":
- from airflow_breeze.utils.console import get_console
+ from airflow_breeze.utils.console import console_print
Review Comment:
`console_print` is already imported at module scope, so importing it again
inside this function is redundant and conflicts with the project guideline to
keep imports at module level. Remove the inner import and use the existing
`console_print`.
##########
dev/breeze/src/airflow_breeze/utils/selective_checks.py:
##########
@@ -1504,7 +1502,7 @@ def get_job_label(self, event_type: str, branch: str):
if "windows-2025" in runner_labels:
continue
if not runner_labels:
- get_console().print("[yellow]No labels found for job
{job_name}.\n", jobs_url)
+ console_print("[yellow]No labels found for job
{job_name}.\n", jobs_url)
return None
Review Comment:
This message contains `{job_name}` but isn't an f-string, so it will print
the literal braces instead of the job name. Interpolate `job_name` (and likely
include `jobs_url` if that's what you intended to show).
##########
dev/breeze/src/airflow_breeze/commands/production_image_commands.py:
##########
@@ -634,12 +634,10 @@ def verify(
perform_environment_checks()
check_remote_ghcr_io_commands()
if image_name and manifest_file:
- get_console().print(
- "[error]You cannot use --image-name and --manifest-file at the
same time. Exiting[/"
- )
+ console_print("[error]You cannot use --image-name and --manifest-file
at the same time. Exiting[/")
Review Comment:
This string ends with an incomplete Rich markup closing tag (`[/`). With
Rich markup enabled, this will raise a MarkupError at runtime. Change it to a
valid closing tag (e.g. `[/]`) or disable markup for this call.
##########
dev/breeze/src/airflow_breeze/utils/path_utils.py:
##########
@@ -447,11 +447,11 @@ def cleanup_python_generated_files():
"You can also remove those files manually using sudo."
)
else:
- get_console().print("[warnings]There were files that you could not
clean-up:\n")
- get_console().print(permission_errors)
- get_console().print("You can also remove those files manually
using sudo.")
+ console_print("[warnings]There were files that you could not
clean-up:\n")
+ console_print(permission_errors)
+ console_print("You can also remove those files manually using
sudo.")
Review Comment:
`[warnings]` is not a valid style tag in the Breeze Rich theme (the theme
defines `warning`, not `warnings`). This may raise a Rich markup/style parsing
error at runtime. Use `[warning]...[/]` instead.
##########
dev/breeze/src/airflow_breeze/commands/main_command.py:
##########
@@ -135,16 +135,14 @@ def check_for_python_emulation():
system_machine = subprocess.check_output(["uname", "-m"],
text=True).strip()
python_machine = platform.uname().machine
if system_machine != python_machine:
- from airflow_breeze.utils.console import get_console
+ from airflow_breeze.utils.console import console_print
- get_console().print(
+ console_print(
Review Comment:
`console_print` is already imported at module scope in this file, so these
in-function imports are redundant and violate the project guideline to avoid
imports inside functions. Remove the inner import(s) and use the module-level
`console_print`.
##########
dev/breeze/src/airflow_breeze/utils/virtualenv_utils.py:
##########
@@ -37,14 +37,14 @@ def create_venv(
capture_output=True,
)
if venv_command_result.returncode != 0:
- get_console().print(
+ console_print(
f"[error]Error when initializing virtualenv in
{venv_path.as_posix()}:[/]\n"
f"{venv_command_result.stdout}\n{venv_command_result.stderr}"
)
sys.exit(venv_command_result.returncode)
python_path = venv_path / "bin" / "python"
if not python_path.exists():
- get_console().print(f"\n[errors]Python interpreter is not exist in
path {python_path}. Exiting!\n")
+ console_print(f"\n[errors]Python interpreter is not exist in path
{python_path}. Exiting!\n")
sys.exit(1)
Review Comment:
`[errors]` is not a valid style tag in the Breeze Rich theme (the theme
defines `error`, not `errors`). This will likely raise a Rich markup/style
parsing error at runtime. Use `[error]...[/]` (or `markup=False`) instead.
##########
dev/breeze/src/airflow_breeze/utils/github.py:
##########
@@ -179,21 +175,21 @@ def get_active_airflow_versions(
for version in airflow_versions:
date = get_tag_date(version)
if not date:
- get_console().print("[error]Error fetching tag date for Airflow
{version}")
+ console_print("[error]Error fetching tag date for Airflow
{version}")
sys.exit(1)
Review Comment:
This message uses `{version}` literally (missing an f-string), so the error
output won't include the actual Airflow version. Convert it to an f-string (or
otherwise interpolate) so it prints the version value.
##########
dev/breeze/src/airflow_breeze/utils/selective_checks.py:
##########
@@ -1491,11 +1489,11 @@ def get_job_label(self, event_type: str, branch: str):
error_msg = jobs_response.json()
except ValueError:
error_msg = jobs_response.text[:200]
- get_console().print(f"[red]Error while listing jobs error:
{error_msg}.\n")
+ console_print(f"[red]Error while listing jobs error:
{error_msg}.\n")
return None
jobs = jobs_response.json().get("jobs", [])
if not jobs:
- get_console().print("[yellow]No jobs information found for jobs
%s.\n", jobs_url)
+ console_print("[yellow]No jobs information found for jobs %s.\n",
jobs_url)
return None
Review Comment:
These console_print calls are passing a format string containing `%s` plus
an extra argument. Rich Console will print them as two separate objects,
leaving `%s` in the output. Interpolate `jobs_url` into the string (e.g. via
f-string) instead of using printf-style placeholders.
##########
dev/breeze/src/airflow_breeze/commands/main_command.py:
##########
@@ -159,9 +157,9 @@ def check_for_python_emulation():
if user_status.upper() not in ["Y", "YES"]:
sys.exit(1)
except TimeoutOccurred:
- from airflow_breeze.utils.console import get_console
+ from airflow_breeze.utils.console import console_print
- get_console().print("\nNo answer, exiting...")
+ console_print("\nNo answer, exiting...")
Review Comment:
`console_print` is already available from the module-level import, so
re-importing it in the exception handler is unnecessary and makes control flow
harder to follow. Use the existing import instead of importing inside the
function body.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
