shahar1 commented on code in PR #76132:
URL: https://github.com/apache/airflow/pull/76132#discussion_r4431846316
##########
airflow-ctl/src/airflowctl/ctl/cli_config.py:
##########
@@ -76,6 +76,8 @@ def _warn_if_write_may_have_applied(error: httpx.HTTPError)
-> None:
"""Warn that a failed write is not retried because the server may already
have applied it."""
if isinstance(error, httpx.HTTPStatusError) and error.response.status_code
< 500:
return
+ if isinstance(error, (httpx.ConnectError, httpx.ConnectTimeout,
httpx.PoolTimeout)):
Review Comment:
**nit** — Testing (AGENTS.md → Testing Standards):
> Target exactly 100% coverage of what the PR changes — no more, no less.
Every changed or added behaviour must have a test; every test must fail without
the PR's change.
`httpx.ConnectError` in this tuple is unreachable: none of the
`safe_call_command` branches that call `_warn_if_write_may_have_applied` catch
it (it is a `NetworkError`, not a
`ReadError`/`WriteError`/`RemoteProtocolError`/`TimeoutException`), and the
parametrized warning test has no `ConnectError` row. `ConnectTimeout` and
`PoolTimeout` are the members that matter, and both are tested. Suggest
dropping it:
```suggestion
if isinstance(error, (httpx.ConnectTimeout, httpx.PoolTimeout)):
```
--
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]
