fix(nemo-deployments): wrap AsyncEntitiesClient via client_from_platform in OpenShell backend - #1077
Draft
maxdubrinsky wants to merge 2 commits into
Conversation
…orm in OpenShell backend OpenShellDeploymentBackend.init() wrapped the SDK's generated AsyncEntitiesResource directly instead of adapting it with client_from_platform(sdk, AsyncEntitiesClient), as the docker and k8s backends do. NemoEntitiesClient.get() forwards a query_params kwarg the generated resource does not accept, so _load_deployment_config raised TypeError: AsyncEntitiesResource.get_entity_by_name() got an unexpected keyword argument 'query_params' and every first reconcile against an openshell executor failed. Adapt the SDK the same way as the docker backend so the wrapped client accepts query_params. Add a regression test that drives the real contract (a live entities client over a mock transport): a 404 surfaces as NemoEntityNotFoundError, which is only reachable once get_entity_by_name(query_params=...) is accepted. The test fails against the old wrapping with the exact query_params TypeError. Update the openshell conftest to patch client_from_platform instead of the removed AsyncEntitiesResource. Fixes AIRCORE-977 Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>
The MissingBackendDependencyError hint told users to run uv sync --package nemo-deployments-plugin --extra openshell, which narrows the workspace venv and uninstalls the platform (AIRCORE-980). Replace it in both the backend and policy modules with the non-destructive uv pip install "openshell>=0.0.92" "grpcio>=1.78.0" "protobuf>=6.31.1". Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>
Contributor
|
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.
Summary
Every deployment to an
openshell-backed executor failed on its first reconcile. Docker-backed deploys of the same config reached READY, isolating the defect to the openshell backend.Two independent fixes, one commit each.
1. Wrong entities client wrapping (root cause,
Fixes AIRCORE-977)OpenShellDeploymentBackend.init()wrapped the SDK's generatedAsyncEntitiesResourcedirectly:NemoEntitiesClient.get()forwards aquery_paramskwarg that the generated resource does not accept, so_load_deployment_configdied with:The docker and k8s backends already do the correct thing: adapt the SDK with
client_from_platform(self._sdk, AsyncEntitiesClient). This change mirrors them:Regression test
Added
test_load_deployment_config_wraps_an_entities_client_that_accepts_query_params, which drives the real contract instead of a hollow constructor assert: it builds the backend against a realAsyncNeMoPlatformSDK backed by an httpxMockTransportthat returns 404, then calls_load_deployment_config. A 404 surfaces asNemoEntityNotFoundError, which is only reachable onceget_entity_by_name(query_params=...)is accepted and the request actually goes out.I verified it is genuinely adversarial: with the source fix reverted the test fails with the exact
TypeError ... unexpected keyword argument 'query_params'; with the fix applied it passes. The openshell conftest was updated to patchclient_from_platforminstead of the removedAsyncEntitiesResource.2. Broken install hint (separate commit)
The
MissingBackendDependencyErrorhint inbackend.pyandpolicy.pytold users to runuv sync --package nemo-deployments-plugin --extra openshell, which narrows the workspace venv and uninstalls the platform (AIRCORE-980). Both copies now use the non-destructive form:Testing
make test-deployments-openshell(installs the openshell extra, runs the backend unit suite): 79 passed.TypeError, passes post-fix.uv run ruff format/uv run ruff checkon all changed files: clean.uv run --frozen ty check plugins/nemo-deployments: no new diagnostics in changed files. The 17 pre-existing diagnostics are unrelated (unused# ty: ignore[unresolved-import]on the openshell imports, which only fire when the openshell extra is installed, plus unrelated pre-existing test typing issues).Fixes AIRCORE-977
https://linear.app/nvidia/issue/AIRCORE-977