fix(deploy-sandbox skill): use non-destructive uv pip install for openshell extras - #1073
Draft
maxdubrinsky wants to merge 2 commits into
Draft
Conversation
…nshell extras Pre-flight step 3 and the recovery table told users to run `uv sync --package nemo-deployments-plugin --extra openshell` to install the openshell extra. `uv sync --package` narrows the workspace venv to that one package's dependency set, which uninstalls nmp, nemo_agents_plugin, and python-on-whales. `nemo services run` then fails with `No module named 'nmp.platform_runner'`, and the step's own DEPS check reports DEPS_MISSING both before and after because the first remedy removes what the second installs. Replace both occurrences with the additive form: uv pip install "openshell>=0.0.92" "grpcio>=1.78.0" "protobuf>=6.31.1" uv pip install -e 'plugins/nemo-agents[container]' `uv pip install` adds the deps into the existing venv without narrowing it, so the platform stays intact. Fixes AIRCORE-980 https://linear.app/nvidia/issue/AIRCORE-980 Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>
Contributor
|
The DEMO.ipynb install cell still ran the destructive uv sync --package nemo-deployments-plugin --extra openshell, which narrows the workspace venv and uninstalls the platform. Replace it with the additive uv pip install form, matching the SKILL.md fix (AIRCORE-980). Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>
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
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.
Root cause
Pre-flight step 3 and the "If verification fails" recovery table of the deploy-sandbox skill told users to install the openshell extra with:
uv sync --packagenarrows the workspace venv to that one package's dependency set. It uninstallsnmp,nemo_agents_plugin, andpython_on_whalesfrom the venv, sonemo services runthen fails withNo module named 'nmp.platform_runner'. The step's own precondition check (import python_on_whales, openshell) reportsDEPS_MISSINGboth before and after, because the first remedy removes exactly what the second (uv pip install -e 'plugins/nemo-agents[container]') reinstalls.Fix
Replace both occurrences of the destructive command with the additive form:
uv pip installadds the two missing pieces into the existing workspace venv without narrowing it, so the platform stays intact. The surrounding prose now spells out whyuv sync --packageis wrong here.Adversarial verification
Run in an isolated worktree venv, proving the new command is non-destructive:
uv sync --frozen --all-packages-> baseline platform venv.python -c 'import nmp.platform_runner'->platform ok;nemo services --helploads its CLI group cleanly..venv/bin/python -c 'import python_on_whales, openshell'->ModuleNotFoundError: No module named 'python_on_whales'(DEPS_MISSING), matching the skill's own check.openshell==0.0.97andpython-on-whales==0.81.0.import python_on_whales, openshell->DEPS_OKANDimport nmp.platform_runner->platform okANDnemo services --helpstill loads.Step 5 is the proof: the new commands add the extras while leaving the platform importable. The old
uv sync --packagecommand was not run against this venv on purpose (it would break the platform, which is the whole bug); the platform-still-imports evidence is the critical half.Fixes AIRCORE-980
https://linear.app/nvidia/issue/AIRCORE-980