Skip to content

common: clean up failed atomic writes - #38552

Merged
adeebshihadeh merged 2 commits into
commaai:masterfrom
GavinnnK:agent/cleanup-atomic-write
Aug 7, 2026
Merged

common: clean up failed atomic writes#38552
adeebshihadeh merged 2 commits into
commaai:masterfrom
GavinnnK:agent/cleanup-atomic-write

Conversation

@GavinnnK

@GavinnnK GavinnnK commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

atomic_write uses delete=False, so an exception from the writer left the partial temp file behind. Always remove it in finally; successful writes still use the same os.replace path.

Adds a regression test for the error path.

Tested on Python 3.12.13. The focused regression, Ruff, py_compile, and git diff --check pass.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Process replay diff report

Replays driving segments through this PR and compares the behavior to master.
Please review any changes carefully to ensure they are expected.

✅ 0 changed, 66 passed, 0 errors

@GavinnnK
GavinnnK force-pushed the agent/cleanup-atomic-write branch from f919d2e to c530648 Compare August 7, 2026 03:22
@GavinnnK

GavinnnK commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

CI note: the two failing jobs both stop in raylib startup because the external runner is missing libGLESv2.so.2. After rebasing onto current master, build release, build macOS, static analysis, and process replay (0 changed / 66 passed) all pass. No failure reaches the atomic_write change or its regression test.

@GavinnnK
GavinnnK marked this pull request as ready for review August 7, 2026 03:44
@GavinnnK
GavinnnK force-pushed the agent/cleanup-atomic-write branch from c530648 to 58dd5da Compare August 7, 2026 19:42
@adeebshihadeh
adeebshihadeh merged commit 0819f5c into commaai:master Aug 7, 2026
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants