[DYNAMO] chore: remove dead unload_lora_adapter#2396
Draft
Conversation
`unload_lora_adapter` has had no callers since 220b50a ("dont unload for now"), which removed the only call site (in orchestrator.py) the same day the function was introduced in #1320. The author intentionally deferred end-of-training adapter unload because the call was unreliable -- the dangling function carried a `# TODO: first one can fail, subsequent ones should succeed` and a commented-out `raise_for_status` that papered over the symptom. Two issues with carrying it as dead code: 1. Silent-failure pattern is exactly what AGENTS.md says not to do. 2. The path string `/v1/unload_lora_adapter` is asymmetric with the bare `/load_lora_adapter` 14 lines above. Against vLLM both work (alias routing); against a Dynamo `/v1/rl/*` admin frontend, the `/v1/` prefix produces `/v1/rl/v1/unload_lora_adapter` which 404s. Deleting rather than fixing -- when end-of-training unload is needed, re-add it correctly with a working integration and a real verification.
AmeenP
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
unload_lora_adapterinsrc/prime_rl/utils/client.pyhas been dead code since November 2025. Removing it.History
2aa47ae6b, 2025-11-21) introducedunload_lora_adapteralong with a call to it at the end oforchestrate()inorchestrator.py. The PR also shipped with a# TODO: first one can fail, subsequent ones should succeedcomment and a commented-outraise_for_status()— i.e. the author already knew the call was unreliable.220b50a38("dont unload for now"), same day, same author, removed the call site and the import. The function has had zero callers ever since.Why delete, not fix
The function has two issues that surface only when reactivated:
raise_for_status()is commented out. PerAGENTS.md: "Minimal try/except: let errors propagate — silent failures hide bugs."/v1/unload_lora_adapter(with/v1/prefix). Line 403 — its siblingload_lora_adapter— is bare/load_lora_adapter. Against vLLM, both paths work because vLLM aliases them. Against a Dynamo frontend withadmin_base_url = http://.../v1/rl, httpx joins the path onto the base, producinghttp://.../v1/rl/v1/unload_lora_adapter, which doesn't exist. The 404 is invisible becauseraise_for_status()is commented out.Fixing in place would mean making dead code "correct" without any way to validate the fix end-to-end. When end-of-training adapter unload is actually needed, the right move is to re-add it with a real call site and a real integration check, not to maintain a half-working stub in the meantime.
Scope
13 lines removed in one file. No callers, no imports, no tests reference it. Verified.