Repository navigation
fix: exit non-zero when system caches delete task fails - #876
pcbeingused333 wants to merge 2 commits into
Conversation
The runtime cache deletion runs as a Connect task that is waited on with raise_on_error=False, and the CLI discarded the returned task, so a task that finished with a non-zero code or an error still produced exit code 0. Raise an RSConnectException in that case so cli_feedback exits with 1. Dry runs return no task and are unaffected. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
karawoo
left a comment
karawoo
left a comment
There was a problem hiding this comment.
Thank you for the contribution @pcbeingused333. I think this doesn't exactly resolve the specific example in #746 but doing so would require some changes on Connect. For other cases where deletion fails and Connect reports an error, this change would catch it appropriately, however I think we should move handling to delete_runtime_cache.
| _, task = ce.delete_runtime_cache(language, version, image_name, dry_run) | ||
| if task is not None and (task["code"] != 0 or task.get("error")): | ||
| reason = task.get("error") or "task exited with status %d" % task["code"] | ||
| raise RSConnectException("Failed to delete the runtime cache: %s" % reason) |
There was a problem hiding this comment.
Instead of handling this here could we instead make the change in delete_runtime_cache? I think we just need to remove raise_on_error=False
rsconnect-python/rsconnect/api.py
Line 3003 in 6afd927
Remove raise_on_error=False so wait_for_task raises on a non-zero exit code, and drop the duplicate check from system_caches_delete. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Makes sense, thanks. Pushed a follow-up: |
Related to #746 (the exact example there also needs changes on Connect).
What
rsconnect system caches deletenow exits with status 1 when the Connect task that deletes the runtime cache finishes with a non-zero code or an error. Before, it printed nothing useful and exited 0, so scripts and CI could not tell that the cache had not been deleted.Why
RSConnectExecutor.delete_runtime_cachewaits for the task withraise_on_error=Falseand returns(result, task), butsystem_caches_deleteinrsconnect/main.pydropped the return value. A failed task therefore never reachedcli_feedback.RSConnectExecutor.delete_runtime_cacheno longer passesraise_on_error=False, sowait_for_taskraisesRSConnectExceptionon a non-zero task code, whichcli_feedbackalready turns into an error message and exit code 1. A dry run returns(result, None)and is unchanged.Tests
Added to
tests/test_main_system_caches.py(using the existing httpretty setup):test_system_caches_delete_task_failed: the task finishes withcode: 1and anerror; expects exit code 1 andTask exited with status 1.in the output. It fails onmain(exit code 0) and passes with this change.test_system_caches_delete_dry_run: no task, exit code 0 (covers the unchanged path).tests/test_main_system_caches.pypasses (9 tests), andruff checkandruff format --checkare clean. The failure path is exercised with httpretty; the change was not run against a live Connect server. The rest of the suite passes (1695 passed in one environment); the only failures seen were environment-dependent and unrelated to this change (test_quickstart.py::test_quickstart_requires_writable_cwdwhen running as root, which fails the same way onmain, and in another environmenttest_environment.py::test_uv_lock_exportand one test intests/test_agent_cli_e2e.py).This change was made with Claude Code.
🤖 Generated with Claude Code