Copilot commented on code in PR #64588:
URL: https://github.com/apache/airflow/pull/64588#discussion_r3025322427
##########
scripts/in_container/install_airflow_and_providers.py:
##########
@@ -1117,6 +1119,50 @@ def install_airflow_and_providers(
console.print("\n[green]Done!")
+def _run_uv_install_with_retry(
+ cmd: list[str],
+ github_actions: bool,
+ check: bool = True,
+ max_attempts: int = 5,
+ sleep_seconds: int = 30,
+) -> subprocess.CompletedProcess:
+ """Run a uv pip install command with retry logic for cargo-related
transient failures.
+
+ Workaround for https://github.com/astral-sh/uv/issues/18801 β uv can fail
with cargo-related
+ errors during package builds. When a failure contains "cargo" or "maturin"
in the output, we retry up to
+ max_attempts times. Non-cargo failures are returned immediately without
retry.
+ """
+ for attempt in range(1, max_attempts + 1):
+ console.print(f"[bright_blue]Attempt {attempt} of {max_attempts} for
uv install")
+ result = run_command(
+ cmd,
+ github_actions=github_actions,
+ check=False,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.STDOUT,
+ )
+ output = result.stdout.decode() if result.stdout else ""
+ if output:
+ console.print(output)
Review Comment:
`stdout=subprocess.PIPE` captures the entire `uv` output in memory and
prevents streaming logs until the command completes. For long `uv pip
install`/wheel builds this can both consume a lot of RAM and risk CI inactivity
timeouts. Consider streaming output while also scanning it (e.g., line-by-line
from a `Popen` pipe, or teeing to a temporary file) so logs remain live and you
don't buffer everything in memory.
```suggestion
# Stream uv output line by line to avoid buffering everything before
printing.
process = subprocess.Popen(
cmd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
text=True,
)
output_lines: list[str] = []
if process.stdout is not None:
for line in process.stdout:
console.print(line.rstrip("\n"))
output_lines.append(line)
returncode = process.wait()
output_text = "".join(output_lines)
result = subprocess.CompletedProcess(
cmd,
returncode,
stdout=output_text.encode(),
stderr=None,
)
output = output_text
```
##########
scripts/docker/install_airflow_when_building_images.sh:
##########
@@ -35,6 +35,45 @@
# shellcheck source=scripts/docker/common.sh
. "$( dirname "${BASH_SOURCE[0]}" )/common.sh"
+# Retry wrapper for uv commands that may fail with transient cargo/maturin
build errors.
+# Workaround for https://github.com/astral-sh/uv/issues/18801
+# Usage: run_uv_with_cargo_retry <command...>
+# Returns the exit code of the command (non-zero only after all retries
exhausted for cargo errors,
+# or immediately for non-cargo failures).
+function run_uv_with_cargo_retry() {
+ local max_attempts=5
+ local sleep_seconds=30
Review Comment:
The helper is named `run_uv_with_cargo_retry`, but itβs also used to wrap
`${PACKAGING_TOOL_CMD} install` which can be `pip` (when `AIRFLOW_USE_UV` is
false). Consider renaming the function to something tool-agnostic (or only
applying it when the underlying command is `uv`) so the name matches actual
behavior.
##########
scripts/in_container/install_airflow_and_providers.py:
##########
@@ -1117,6 +1119,50 @@ def install_airflow_and_providers(
console.print("\n[green]Done!")
+def _run_uv_install_with_retry(
+ cmd: list[str],
+ github_actions: bool,
+ check: bool = True,
+ max_attempts: int = 5,
+ sleep_seconds: int = 30,
+) -> subprocess.CompletedProcess:
+ """Run a uv pip install command with retry logic for cargo-related
transient failures.
+
+ Workaround for https://github.com/astral-sh/uv/issues/18801 β uv can fail
with cargo-related
+ errors during package builds. When a failure contains "cargo" or "maturin"
in the output, we retry up to
+ max_attempts times. Non-cargo failures are returned immediately without
retry.
+ """
+ for attempt in range(1, max_attempts + 1):
+ console.print(f"[bright_blue]Attempt {attempt} of {max_attempts} for
uv install")
+ result = run_command(
+ cmd,
+ github_actions=github_actions,
+ check=False,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.STDOUT,
+ )
+ output = result.stdout.decode() if result.stdout else ""
+ if output:
+ console.print(output)
Review Comment:
`console.print(output)` will parse Rich markup by default; command output
commonly contains bracketed tokens (e.g. pip/uv `[notice] ...`) which can be
mis-rendered or potentially raise markup parsing errors. Print captured command
output with `markup=False` (and typically `highlight=False`) to ensure raw
output is displayed safely.
```suggestion
console.print(output, markup=False, highlight=False)
```
##########
scripts/in_container/install_airflow_and_providers.py:
##########
@@ -1117,6 +1119,50 @@ def install_airflow_and_providers(
console.print("\n[green]Done!")
+def _run_uv_install_with_retry(
+ cmd: list[str],
+ github_actions: bool,
+ check: bool = True,
+ max_attempts: int = 5,
+ sleep_seconds: int = 30,
+) -> subprocess.CompletedProcess:
+ """Run a uv pip install command with retry logic for cargo-related
transient failures.
+
+ Workaround for https://github.com/astral-sh/uv/issues/18801 β uv can fail
with cargo-related
+ errors during package builds. When a failure contains "cargo" or "maturin"
in the output, we retry up to
+ max_attempts times. Non-cargo failures are returned immediately without
retry.
+ """
+ for attempt in range(1, max_attempts + 1):
+ console.print(f"[bright_blue]Attempt {attempt} of {max_attempts} for
uv install")
+ result = run_command(
+ cmd,
+ github_actions=github_actions,
+ check=False,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.STDOUT,
+ )
+ output = result.stdout.decode() if result.stdout else ""
+ if output:
+ console.print(output)
+ if result.returncode == 0:
+ return result
+ if "cargo" not in output and "maturin" not in output:
+ console.print("[yellow]Failure is not cargo-related, not
retrying.")
Review Comment:
`result.stdout.decode()` uses the default UTF-8 decode without an error
handler and the cargo detection is case-sensitive. To avoid crashes on
unexpected byte sequences and to match logs that might contain
`Cargo`/`Maturin`, decode with an explicit error strategy (e.g.
`errors='replace'`) and normalize casing (e.g. compare against
`output.lower()`).
--
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]
