You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "ericdelorefice (via GitHub)" <gi...@apache.org> on 2026/10/18 11:07:10 UTC
[PR] Fail closed in WASB log handler when existing-log read fails [airflow]
ericdelorefice opened a new pull request, #74996:
URL: https://github.com/apache/airflow/pull/74996
This brings the WASB remote log handler to the contract the GCS handler adopted in #67511: when an append-mode upload cannot read the log already stored in the blob, it keeps the local log and returns `False` instead of uploading over the blob with only the new content.
### Problem
`WasbRemoteLogIO.write(log, remote_log_location, append=True)` checks whether the blob exists, reads it, and uploads old plus new content with `overwrite=True`. Both helpers it calls swallow errors: `wasb_log_exists` returns `False` on any exception, and `wasb_read` returns `""` on any exception. So when either request fails for a reason other than "blob not found" (a throttled request, a timeout, a network error), `write` treats the stored log as empty, replaces the blob with only the new segment and returns `True`. `upload()` then truncates the local file, because the write reported success. The earlier part of the attempt log is gone from both places.
One attempt log is uploaded many times in some lifecycles. #70867 describes the clearest case: a reschedule-mode sensor writes every poke to the same `attempt=N.log`. One failed read during any poke drops every earlier poke from the stored log.
### Reproduction
The real `WasbRemoteLogIO` from `main` (79ce98f110), the real `WasbHook` and `azure-storage-blob` 12.31.0, against Azurite (the Azure Storage emulator) holding the blob. The failure is injected: a small local proxy in front of Azurite answers requests for the log blob with HTTP 503 `ServerBusy`, which is what Azure Storage returns when it throttles, and azure-core retries three times before raising. I have not seen this in a user report. Nothing in Airflow or the Azure SDK is patched.
Four pokes of a reschedule-mode sensor, each appending a line locally and calling `upload("attempt=1.log")`; poke 3 is throttled.
On `main`, with the GET (read) throttled:
```
--- after poke 2
remote blob: 'poke 1: ...\npoke 2: ...\n'
[error] Could not read logs from dag_id=d/run_id=r/task_id=sensor/attempt=1.log
azure.core.exceptions.HttpResponseError: ... ErrorCode:ServerBusy
--- after poke 3 (GET throttled)
remote blob: 'poke 3: ...\n'
local file : ''
--- after poke 4
remote blob: 'poke 3: ...\npoke 4: ...\n'
```
With the HEAD (existence check) throttled instead, the result is the same, and nothing is logged above debug level, because `wasb_log_exists` logs the error at debug and returns `False`.
With this change, both runs keep the stored log and the local copy, and the next poke uploads everything:
```
[warning] Refusing to overwrite remote log dag_id=d/run_id=r/task_id=sensor/attempt=1.log: could not read existing content (... ErrorCode:ServerBusy). Keeping local logs for later retry.
--- after poke 3 (GET throttled)
remote blob: 'poke 1: ...\npoke 2: ...\n'
local file : 'poke 3: ...\n'
--- after poke 4
remote blob: 'poke 1: ...\npoke 2: ...\npoke 3: ...\npoke 4: ...\n'
local file : ''
```
### Change
`write` now calls `hook.check_for_blob` and `hook.read_file` directly, inside one `try`. `check_for_blob` already returns `False` only for `ResourceNotFoundError` and raises for anything else, so a missing blob is still written as a new blob. Any other failure logs a warning naming the blob and the error and returns `False` without uploading, the same message and contract as the GCS handler. `append=False` is unchanged. `wasb_log_exists` and `wasb_read` are left as they are; nothing else in the provider calls them.
Intended behaviour change: an append-mode upload that cannot read the existing log no longer uploads. The local log stays on the worker, and a later upload of the same attempt sends it. If there is no later upload, it stays only on the worker, the same trade-off #67511 made for GCS. A worker whose credentials can write blobs but cannot read them now keeps its logs locally instead of overwriting the blob on every upload.
The S3 handler has the same read-side gap (`s3_read` returns `""` on error, and `write` appends to that). I will send that separately, since #73517 is changing the S3 read path.
### Testing
macOS, Python 3.12.14, plain `pytest` (not Breeze) with `apache-airflow` 3.3.2 from PyPI and this provider installed editable from the branch.
`providers/microsoft/azure/tests/unit/microsoft/azure/log/test_wasb_task_handler.py`:
- 3 new tests fail on `main` and pass with this change: `write` returns `False` and does not call `load_string` when the existence check fails and when the read fails, and the warning names the blob and carries the storage error; and an `upload()` sequence where the third read fails leaves the stored blob and the local file untouched, then the fourth upload stores all four segments and truncates the local file.
- 1 new control passes both ways: a blob that does not exist is written with the new content, and `read_file` is not called.
- 3 existing tests (`test_write_log`, `test_write_on_existing_log`, `test_write_on_existing_log_already_newline_terminated`) patched `wasb_log_exists` and `wasb_read`; they now set the hook's `check_for_blob` and `read_file` instead, with the same expectations, and pass both ways.
- With the source reverted to `main` and the new tests kept, the 3 fail and the controls pass, five runs each way.
- Whole file: 31 passed on `main`, 35 passed on this branch.
- `prek run --files` on both files: all 46 hooks that apply passed (ruff, ruff format, codespell, the provider import checks, lazy logging and the rest). The `mypy-providers` pre-push hook needs breeze, which needs Docker, and I don't have Docker on this machine, so I ran `mypy` with the repo's `pyproject.toml` configuration in the Azure provider's uv environment instead: no issues in either file.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code (Claude Opus 5.5) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fail closed in WASB log handler when existing-log read fails [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #74996:
URL: https://github.com/apache/airflow/pull/74996#issuecomment-6249116435
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our [Contributors' Guide](https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:
- Pay attention to the quality of your code (ruff, mypy and type annotations). Our [prek-hooks]( https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-prek-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/airflow-core/docs/howto/custom-operator.rst) Consider adding an example Dag that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/dev/breeze/doc/README.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#coding-style-and-best-practices).
- Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org