Visitar URL original
test: Retry CliRunner apply subprocess on timeout to reduce flakiness · feast-dev/feast@e74df8e · GitHub
Skip to content

Commit e74df8e

Browse files
nikolauspschuetzntkathole
authored andcommitted
test: Retry CliRunner apply subprocess on timeout to reduce flakiness
CliRunner.run() (used for 'feast apply' in unit tests) spawns a fresh child interpreter that must import Feast (pandas/pyarrow/...) from cold. On loaded CI runners — observed on unit-test-python (3.11, macos-14) — this can exceed the 120s timeout, failing the test with: AssertionError: Command timed out after 120s: ['apply'] Unlike the materializing paths guarded by run_with_output(), commands routed through run() do not materialize and so cannot trigger the Dask atexit hang the timeout was added for. A timeout here is therefore almost always transient cold -start slowness, not a genuine deadlock, so retrying once (with warm OS/page caches) clears it while a real repeated stall still fails after the attempts are exhausted. Builds on the CliRunner subprocess stabilization in #6560. Signed-off-by: Nikolaus Schuetz <nikolauspschuetz@gmail.com>
1 parent 3f6359d commit e74df8e

1 file changed

Lines changed: 33 additions & 16 deletions

File tree

‎sdk/python/tests/utils/cli_repo_creator.py‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -63,26 +63,43 @@ def _subprocess_env(self) -> dict[str, str]:
6363
env["PYTHONPATH"] = os.pathsep.join(parent_paths)
6464
return env
6565

66-
def run(self, args: List[str], cwd: Path) -> subprocess.CompletedProcess:
66+
def run(
67+
self, args: List[str], cwd: Path, attempts: int = 2
68+
) -> subprocess.CompletedProcess:
6769
# Apply a conservative timeout to prevent CI hangs from Dask atexit-handler
6870
# stalls or other subprocess blockages.
6971
timeout = 120
7072

71-
try:
72-
return subprocess.run(
73-
[sys.executable, cli.__file__] + args,
74-
cwd=cwd,
75-
capture_output=True,
76-
timeout=timeout,
77-
env=self._subprocess_env(),
78-
)
79-
except subprocess.TimeoutExpired:
80-
return subprocess.CompletedProcess(
81-
args=[sys.executable, cli.__file__] + args,
82-
returncode=-1,
83-
stdout=b"",
84-
stderr=f"Command timed out after {timeout}s: {args}".encode(),
85-
)
73+
# Commands routed through here (e.g. `apply`) do not materialize, so they
74+
# cannot trigger the Dask atexit hang described in the module docstring. A
75+
# timeout is therefore almost always transient slowness — a cold child
76+
# interpreter importing Feast (pandas/pyarrow/...) on a loaded CI runner,
77+
# which has been observed to exceed the timeout on the slower macOS
78+
# runners. Retry once so a single slow cold start does not flake the test:
79+
# the retry runs with warm OS/page caches and typically finishes quickly,
80+
# while a genuine repeated stall still fails once attempts are exhausted.
81+
full_args = [sys.executable, cli.__file__] + args
82+
result = None
83+
for attempt in range(1, attempts + 1):
84+
try:
85+
return subprocess.run(
86+
full_args,
87+
cwd=cwd,
88+
capture_output=True,
89+
timeout=timeout,
90+
env=self._subprocess_env(),
91+
)
92+
except subprocess.TimeoutExpired:
93+
result = subprocess.CompletedProcess(
94+
args=full_args,
95+
returncode=-1,
96+
stdout=b"",
97+
stderr=(
98+
f"Command timed out after {timeout}s "
99+
f"(attempt {attempt}/{attempts}): {args}"
100+
).encode(),
101+
)
102+
return result
86103

87104
def run_with_output(self, args: List[str], cwd: Path) -> Tuple[int, bytes]:
88105
is_teardown = "teardown" in args

0 commit comments

Comments
 (0)