feat(files): Migrate consumers to the new FilesClient - #584
Merged
matthewgrossman merged 15 commits intoJul 8, 2026
Merged
Conversation
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Contributor
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR migrates fileset operations from legacy SDK files APIs to plugin ChangesFiles client migration
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/file_manager.py (1)
166-184: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle
ConflictErrorwhen creating the fileset.create_filesetdoesn’t supportexist_ok; if another process wins the race, catchConflictErrorand treat the fileset as already present.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/file_manager.py` around lines 166 - 184, _handle_storage validation in _validate_storage should tolerate a race when create_fileset is called. Update the FileManager._validate_storage flow so that the create_fileset call for both AsyncFilesClient and FilesClient catches ConflictError and treats it as an already-existing fileset instead of failing, while leaving the FileNotFoundError and FileStorageDoesNotExist behavior unchanged.
🧹 Nitpick comments (9)
plugins/nemo-data-designer/tests/integration/test_personas_cli.py (1)
260-266: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMock created without
spec.
Mock()for the patchedFilesClienthas nospec, so a typo'd method name (e.g. wrong signature oncreate_fileset) wouldn't be caught by the test. ConsiderMock(spec=FilesClient).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/nemo-data-designer/tests/integration/test_personas_cli.py` around lines 260 - 266, The mocked FilesClient in test_make_fileset_create_fileset_internal_error_surfaces_clearly is created without a spec, so the test won’t catch typos or invalid method usage. Update the mock setup to use a spec based on FilesClient when creating mock_files, keeping the existing create_fileset side effect and patch of client_from_platform intact so the test still exercises the same error path with interface validation.e2e/files/test_storage_backends.py (1)
47-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAPI usage matches new contract.
create_fileset/delete_filesetcalls and storage config wiring line up with the plugin endpoint signatures. No functional issues.Minor DRY note:
files = client_from_platform(sdk, FilesClient)is instantiated separately inngc_fileset,hf_fileset,test_create_error,test_create_error_nonexistent_secret, andtest_error_nonexistent_repo. A sharedfiles_clientfixture would cut the repetition.♻️ Suggested fixture
+@pytest.fixture +def files_client(sdk: NeMoPlatform) -> FilesClient: + return client_from_platform(sdk, FilesClient)Also applies to: 93-115, 206-241, 297-308
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/files/test_storage_backends.py` around lines 47 - 68, The repeated FilesClient setup is duplicated across ngc_fileset, hf_fileset, test_create_error, test_create_error_nonexistent_secret, and test_error_nonexistent_repo, so extract client_from_platform(sdk, FilesClient) into a shared files_client fixture and update those helpers/tests to depend on it instead of creating the client locally.packages/data_designer_nemo/src/data_designer_nemo/person_reader.py (1)
23-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocstring is stale. It claims
FilesetFileSystemflips toasynchronous=Truewhen handed anAsyncNeMoPlatform, but it now takes aFilesClient, not an SDK. Update to describe the sync-SDK→FilesClientpath.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/data_designer_nemo/src/data_designer_nemo/person_reader.py` around lines 23 - 33, The docstring in the person reader is stale and still describes passing an AsyncNeMoPlatform into FilesetFileSystem, but the current flow uses a sync SDK to build a FilesClient instead. Update the text around the person_reader construction path to match the new sync-SDK→FilesClient behavior, keeping the explanation aligned with the FilesetFileSystem and FilesClient symbols.plugins/nemo-evaluator/tests/api/service/test_metric_service.py (1)
35-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
_FakeAsyncFilesClientacross 4 test files.Identical (or near-identical) fake defined in
test_metrics_routes.py,test_metric_refs.py, andtest_metric_storage.py. Consider extracting to a shared conftest/fixture module.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/nemo-evaluator/tests/api/service/test_metric_service.py` around lines 35 - 54, The _FakeAsyncFilesClient test double is duplicated across multiple metric test modules, so extract the shared fake into a common test helper or conftest fixture and reuse it from test_metric_service alongside test_metrics_routes, test_metric_refs, and test_metric_storage. Keep the behavior in _FakeAsyncFilesClient identical, but centralize the class definition so future changes to create_fileset, delete_fileset, upload_file, and download_file only need to be made once.plugins/nemo-evaluator/tests/api/v2/test_metrics_routes.py (1)
55-62: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInline
_Respclass is inconsistent with sibling test fakes.Other test files (e.g.
test_metric_storage.py) use a top-level_FakeResponse(data)class. Prefer reusing that shape here for consistency instead of defining a nested class with a manually-assigned_dataattribute.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/nemo-evaluator/tests/api/v2/test_metrics_routes.py` around lines 55 - 62, The inline _Resp fake in download_file is inconsistent with the shared test fake shape used elsewhere. Replace the nested class in download_file with the existing top-level _FakeResponse(data) pattern (or equivalent shared fake) and return it directly using the stored bytes for the requested (workspace, name, path), so the test helpers stay consistent across files.packages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/file_manager.py (1)
144-180: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate sync/async branching + redundant client construction.
isinstance(self.sdk, AsyncNeMoPlatform)branching is duplicated between__post_init__(lines 145-148) and_validate_storage(lines 173-179)._validate_storagealso constructs a brand-new client viaclient_from_platforminstead of reusing the one already built in__post_init__and stored insideself._fs.♻️ Extract a shared helper
+ def _files_client(self) -> "AsyncFilesClient | FilesClient": + if isinstance(self.sdk, AsyncNeMoPlatform): + return client_from_platform(self.sdk, AsyncFilesClient) + return client_from_platform(self.sdk, FilesClient) + def __post_init__(self): - if isinstance(self.sdk, AsyncNeMoPlatform): - files_client = client_from_platform(self.sdk, AsyncFilesClient) - else: - files_client = client_from_platform(self.sdk, FilesClient) - self._fs = FilesetFileSystem(client=files_client) + self._fs = FilesetFileSystem(client=self._files_client())logger.info(f"Creating new fileset: [{self.fileset_name}] in workspace [{self.workspace}]") - if isinstance(self.sdk, AsyncNeMoPlatform): - files = client_from_platform(self.sdk, AsyncFilesClient) - await files.create_fileset( - body=CreateFilesetRequest(name=self.fileset_name), workspace=self.workspace - ) - else: - files = client_from_platform(self.sdk, FilesClient) - files.create_fileset(body=CreateFilesetRequest(name=self.fileset_name), workspace=self.workspace) + files = self._files_client() + if isinstance(self.sdk, AsyncNeMoPlatform): + await files.create_fileset( + body=CreateFilesetRequest(name=self.fileset_name), workspace=self.workspace + ) + else: + files.create_fileset(body=CreateFilesetRequest(name=self.fileset_name), workspace=self.workspace)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/file_manager.py` around lines 144 - 180, The sync/async client selection is duplicated in __post_init__ and _validate_storage, and _validate_storage is recreating a new platform client instead of reusing the one already initialized through FilesetFileSystem. Extract the AsyncNeMoPlatform/FilesClient vs AsyncFilesClient selection into a shared helper or reuse the existing client from self._fs in FileManager, then update __post_init__ and _validate_storage to call that shared path so the branching and client construction live in one place.services/core/jobs/tests/test_dispatcher.py (1)
647-698: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate mock-files-client boilerplate across tests.
The same ~8-line block (
mock_files = AsyncMock(),mock_fileset_obj,mock_resp.data.return_value,patch("...client_from_platform", ...)) is repeated 3 times in this file and again intest_job_logs.py'sdispatcherfixture. Consider extracting a small helper (e.g.make_mock_files_client()) shared via conftest to cut duplication.♻️ Example helper
def make_mock_files_client(fileset_name: str = "test-fileset-id") -> AsyncMock: mock_files = AsyncMock() mock_fileset_obj = MagicMock() mock_fileset_obj.name = fileset_name mock_resp = MagicMock() mock_resp.data.return_value = mock_fileset_obj mock_files.create_fileset.return_value = mock_resp return mock_filesAlso applies to: 778-787, 817-868
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/core/jobs/tests/test_dispatcher.py` around lines 647 - 698, The mock files-client setup is duplicated across multiple dispatcher tests, including the repeated AsyncMock/MagicMock create_fileset boilerplate and the client_from_platform patch. Extract that setup into a small shared helper such as make_mock_files_client() in a common test utility or conftest, then reuse it from the dispatcher tests and the test_job_logs fixture to keep the test code DRY. Use the existing dispatcher-related test helpers and the client_from_platform patch point as the main places to update.services/core/files/tests/integration/conftest.py (1)
170-173: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a shared
files_clientfixture.
client_from_platform(sdk, FilesClient)is re-created inline across almost every test file in this cohort (test_ngc_storage.py, test_files_sdk.py, test_filesets_allowed_hosts.py, tests_filesets_with_auth_secrets.py). A single fixture here would cut repeated boilerplate.♻️ Proposed fixture
+@pytest.fixture +def files_client(sdk: NeMoPlatform) -> FilesClient: + return client_from_platform(sdk, FilesClient) + + def fileset_cleanup(sdk: NeMoPlatform) -> Iterator[Callable[[str], None]]:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/core/files/tests/integration/conftest.py` around lines 170 - 173, Create a shared `files_client` fixture in `conftest.py` instead of constructing `client_from_platform(sdk, FilesClient)` inline in each test file. Update the existing cleanup/setup flow around `to_cleanup` and the `files` client usage so tests can depend on the fixture directly, and then replace the repeated client creation in the affected tests with that fixture to remove boilerplate and centralize client setup.services/automodel/src/nmp/automodel/tasks/file_io/run.py (1)
391-436: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDuplicate fileset-create/conflict-patch logic across services.
_create_fileset_with_retry(and its retry policy) is duplicated near-verbatim inservices/rl/src/nmp/rl/tasks/file_io/run.py, and likelyservices/unsloth/.../file_io/run.pyper the PR stack. Worth extracting into a shared helper (e.g. innmp_common) so retry/exception handling for fileset creation doesn't drift across services.Also note:
passthrough=(ConflictError,)oncreate_fileset'ssdk_error_handler(line 391) is now dead —ConflictErroris always swallowed inside_create_fileset_with_retryand never propagates.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/automodel/src/nmp/automodel/tasks/file_io/run.py` around lines 391 - 436, The fileset-create retry/conflict handling in _create_fileset_with_retry is duplicated across services and the ConflictError passthrough in the surrounding sdk_error_handler is no longer used. Move the shared create/patch/retry behavior into a common helper (for example in nmp_common) and update callers like _create_fileset_with_retry to use it. Then remove ConflictError from the passthrough on create_fileset, since ConflictError is already handled internally by the helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/nemo-data-designer/tests/integration/test_personas_cli.py`:
- Around line 80-86: The FilesClient pagination usage in the persona CLI
integration test should rely on the page’s data collection rather than a
non-existent items() API. Update the assertions around files.list_filesets in
test_personas_cli.py to use the returned page’s data() results consistently, and
keep the existing FilesClient and get_fileset checks unchanged.
In `@plugins/nemo-safe-synthesizer/scripts/setup_model_filesets.py`:
- Around line 108-114: The fileset lookup in create_filesets only handles
NotFoundError, so other NemoHTTPError subclasses can escape and stop the batch.
Update the try/except around files.get_fileset to catch the broader
NemoHTTPError as well, matching the create_fileset error handling, and keep the
existing flow that logs the issue and continues processing the remaining
filesets.
In `@services/core/jobs/tests/conftest.py`:
- Around line 246-259: Add the missing pytest fixture decoration to
_mock_files_client in conftest so mock_nmp_client can resolve it as a fixture
dependency. Keep the existing helper behavior intact, but make
_mock_files_client discoverable by pytest alongside mock_nmp_client.
In `@services/core/models/tests/integration/test_chat_template_tool_calling.py`:
- Around line 191-201: The `updated_fileset` mock used in the integration test
is missing a `storage` attribute, which causes `analyze_checkpoint` to fail when
it inspects the returned fileset. Update the `updated_fileset` setup in the test
to include a `storage` value compatible with the code path in
`analyze_checkpoint` (the logic that checks `fs.storage` against storage config
types). Keep the mock aligned with what `get_fileset().data()` returns so the
test reaches the `sdk.models.update` flow instead of raising `AttributeError`.
---
Outside diff comments:
In `@packages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/file_manager.py`:
- Around line 166-184: _handle_storage validation in _validate_storage should
tolerate a race when create_fileset is called. Update the
FileManager._validate_storage flow so that the create_fileset call for both
AsyncFilesClient and FilesClient catches ConflictError and treats it as an
already-existing fileset instead of failing, while leaving the FileNotFoundError
and FileStorageDoesNotExist behavior unchanged.
---
Nitpick comments:
In `@e2e/files/test_storage_backends.py`:
- Around line 47-68: The repeated FilesClient setup is duplicated across
ngc_fileset, hf_fileset, test_create_error,
test_create_error_nonexistent_secret, and test_error_nonexistent_repo, so
extract client_from_platform(sdk, FilesClient) into a shared files_client
fixture and update those helpers/tests to depend on it instead of creating the
client locally.
In `@packages/data_designer_nemo/src/data_designer_nemo/person_reader.py`:
- Around line 23-33: The docstring in the person reader is stale and still
describes passing an AsyncNeMoPlatform into FilesetFileSystem, but the current
flow uses a sync SDK to build a FilesClient instead. Update the text around the
person_reader construction path to match the new sync-SDK→FilesClient behavior,
keeping the explanation aligned with the FilesetFileSystem and FilesClient
symbols.
In `@packages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/file_manager.py`:
- Around line 144-180: The sync/async client selection is duplicated in
__post_init__ and _validate_storage, and _validate_storage is recreating a new
platform client instead of reusing the one already initialized through
FilesetFileSystem. Extract the AsyncNeMoPlatform/FilesClient vs AsyncFilesClient
selection into a shared helper or reuse the existing client from self._fs in
FileManager, then update __post_init__ and _validate_storage to call that shared
path so the branching and client construction live in one place.
In `@plugins/nemo-data-designer/tests/integration/test_personas_cli.py`:
- Around line 260-266: The mocked FilesClient in
test_make_fileset_create_fileset_internal_error_surfaces_clearly is created
without a spec, so the test won’t catch typos or invalid method usage. Update
the mock setup to use a spec based on FilesClient when creating mock_files,
keeping the existing create_fileset side effect and patch of
client_from_platform intact so the test still exercises the same error path with
interface validation.
In `@plugins/nemo-evaluator/tests/api/service/test_metric_service.py`:
- Around line 35-54: The _FakeAsyncFilesClient test double is duplicated across
multiple metric test modules, so extract the shared fake into a common test
helper or conftest fixture and reuse it from test_metric_service alongside
test_metrics_routes, test_metric_refs, and test_metric_storage. Keep the
behavior in _FakeAsyncFilesClient identical, but centralize the class definition
so future changes to create_fileset, delete_fileset, upload_file, and
download_file only need to be made once.
In `@plugins/nemo-evaluator/tests/api/v2/test_metrics_routes.py`:
- Around line 55-62: The inline _Resp fake in download_file is inconsistent with
the shared test fake shape used elsewhere. Replace the nested class in
download_file with the existing top-level _FakeResponse(data) pattern (or
equivalent shared fake) and return it directly using the stored bytes for the
requested (workspace, name, path), so the test helpers stay consistent across
files.
In `@services/automodel/src/nmp/automodel/tasks/file_io/run.py`:
- Around line 391-436: The fileset-create retry/conflict handling in
_create_fileset_with_retry is duplicated across services and the ConflictError
passthrough in the surrounding sdk_error_handler is no longer used. Move the
shared create/patch/retry behavior into a common helper (for example in
nmp_common) and update callers like _create_fileset_with_retry to use it. Then
remove ConflictError from the passthrough on create_fileset, since ConflictError
is already handled internally by the helper.
In `@services/core/files/tests/integration/conftest.py`:
- Around line 170-173: Create a shared `files_client` fixture in `conftest.py`
instead of constructing `client_from_platform(sdk, FilesClient)` inline in each
test file. Update the existing cleanup/setup flow around `to_cleanup` and the
`files` client usage so tests can depend on the fixture directly, and then
replace the repeated client creation in the affected tests with that fixture to
remove boilerplate and centralize client setup.
In `@services/core/jobs/tests/test_dispatcher.py`:
- Around line 647-698: The mock files-client setup is duplicated across multiple
dispatcher tests, including the repeated AsyncMock/MagicMock create_fileset
boilerplate and the client_from_platform patch. Extract that setup into a small
shared helper such as make_mock_files_client() in a common test utility or
conftest, then reuse it from the dispatcher tests and the test_job_logs fixture
to keep the test code DRY. Use the existing dispatcher-related test helpers and
the client_from_platform patch point as the main places to update.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 03f7cb33-51d6-4ff1-9f16-3cf9bacee16f
⛔ Files ignored due to path filters (5)
sdk/python/nemo-platform/src/nemo_platform/cli/commands/api/files/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/api/files/filesets.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/filesets/filesystem/filesystem.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/filesets/resources.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/integration/test_filesets.pyis excluded by!sdk/**
📒 Files selected for processing (84)
e2e/files/test_files.pye2e/files/test_storage_backends.pye2e/test_data_designer.pye2e/test_jobs_auth.pypackages/data_designer_nemo/src/data_designer_nemo/fileset_file_seed_reader.pypackages/data_designer_nemo/src/data_designer_nemo/nemotron_personas.pypackages/data_designer_nemo/src/data_designer_nemo/person_reader.pypackages/data_designer_nemo/src/data_designer_nemo/person_sampling.pypackages/data_designer_nemo/src/data_designer_nemo/seed.pypackages/data_designer_nemo/tests/unit/test_fileset_file_seed_reader.pypackages/data_designer_nemo/tests/unit/test_person_sampling.pypackages/filesets/src/filesets/filesystem/filesystem.pypackages/filesets/src/filesets/resources.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/files/__init__.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/files/filesets.pypackages/nemo_platform_ext/tests/cli/integration/test_filesets.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/file_manager.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/scheduler.pypackages/nmp_common/tests/jobs/conftest.pypackages/nmp_common/tests/jobs/test_file_manager.pypackages/nmp_customization_common/src/nmp/customization_common/service/platform_client.pyplugins/nemo-data-designer/src/nemo_data_designer_plugin/testing/utils.pyplugins/nemo-data-designer/tests/integration/test_personas_cli.pyplugins/nemo-evaluator/examples/plugin_examples.pyplugins/nemo-evaluator/src/nemo_evaluator/filesets.pyplugins/nemo-evaluator/src/nemo_evaluator/metric_storage.pyplugins/nemo-evaluator/tests/api/service/test_metric_service.pyplugins/nemo-evaluator/tests/api/v2/test_metrics_routes.pyplugins/nemo-evaluator/tests/test_filesets.pyplugins/nemo-evaluator/tests/test_metric_refs.pyplugins/nemo-evaluator/tests/test_metric_storage.pyplugins/nemo-safe-synthesizer/scripts/setup_model_filesets.pyplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/api/v2/jobs/endpoints.pyplugins/nemo-safe-synthesizer/tests/unit/test_jobs.pyservices/automodel/src/nmp/automodel/tasks/file_io/run.pyservices/automodel/src/nmp/automodel/tasks/model_entity/run.pyservices/automodel/tests/test_compiler.pyservices/core/entities/src/nmp/core/entities/controllers/workspace_cleanup.pyservices/core/entities/tests/controllers/test_workspace_cleanup.pyservices/core/files/script/v2_migration.pyservices/core/files/src/nmp/core/files/testing/utils.pyservices/core/files/tests/integration/conftest.pyservices/core/files/tests/integration/external_storage/test_huggingface_storage.pyservices/core/files/tests/integration/external_storage/test_ngc_storage.pyservices/core/files/tests/integration/external_storage/test_s3_storage.pyservices/core/files/tests/integration/test_files_basic.pyservices/core/files/tests/integration/test_files_sdk.pyservices/core/files/tests/integration/test_fileset_filesystem.pyservices/core/files/tests/integration/test_filesets_allowed_hosts.pyservices/core/files/tests/integration/test_huggingface_endpoints.pyservices/core/files/tests/integration/test_otlp_endpoints.pyservices/core/files/tests/integration/tests_filesets_with_auth_secrets.pyservices/core/jobs/src/nmp/core/jobs/app/dispatcher.pyservices/core/jobs/tests/conftest.pyservices/core/jobs/tests/test_dispatcher.pyservices/core/jobs/tests/test_job_logs.pyservices/core/jobs/tests/test_jobs_api.pyservices/core/models/src/nmp/core/models/api/permissions.pyservices/core/models/src/nmp/core/models/api/service/model_entity_service.pyservices/core/models/src/nmp/core/models/tasks/model_spec/run.pyservices/core/models/tests/integration/test_chat_template_tool_calling.pyservices/core/models/tests/integration/test_model_entity_service_integration.pyservices/core/models/tests/integration/test_models_with_auth.pyservices/core/models/tests/integration/test_workspace_iam_models_isolation.pyservices/core/models/tests/unit/test_model_entity_service_unit.pyservices/hello-world/src/nmp/hello_world/tasks/access_fileset/run.pyservices/rl/src/nmp/rl/tasks/file_io/run.pyservices/rl/src/nmp/rl/tasks/model_entity/run.pyservices/unsloth/src/nmp/unsloth/tasks/file_io/run.pyservices/unsloth/src/nmp/unsloth/tasks/model_entity/run.pyservices/unsloth/tests/test_file_io.pyservices/unsloth/tests/test_model_entity.pytests/agentic-use/customizer-lora-job-cli/tests/test_outputs.pytests/agentic-use/evaluator-llm-judge-cli-easy/tests/test_outputs.pytests/agentic-use/evaluator-llm-judge-cli/tests/test_outputs.pytests/agentic-use/evaluator-simple-job-cli-easy/tests/test_outputs.pytests/agentic-use/evaluator-simple-job-cli/tests/test_outputs.pytests/agentic-use/evaluator-tool-calling-cli/tests/test_outputs.pytests/agentic-use/evaluator-zero-config-judge-cli/tests/test_outputs.pytests/agentic-use/files-crud-cli-easy/tests/test_outputs.pytests/agentic-use/files-crud-cli/tests/test_outputs.pytests/agentic-use/files-upload-dataset-cli-easy/tests/test_outputs.pytests/agentic-use/files-upload-dataset-cli/tests/test_outputs.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/cli_generator/overrides/files/upload.py
💤 Files with no reviewable changes (1)
- services/automodel/tests/test_compiler.py
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
…s-from-filesetssubresource-to Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Tests that mock the NeMoPlatform SDK need to account for client_from_platform accessing _custom_headers and _client.headers. Patch the adapter function in test_schema, test_jobs, and test_models_api to return mock FilesClient instances. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
These error aliases from nemo_platform_plugin.client.errors are not files-specific — they come from the generic typed HTTP client. Rename to Client* to reflect that. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
…s-from-filesetssubresource-to Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
5 tasks
…s-from-filesetssubresource-to Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
maxdubrinsky
approved these changes
Jul 8, 2026
matthewgrossman
deleted the
mgrossman/aircore-840-migrate-files-consumers-from-filesetssubresource-to
branch
July 8, 2026 22:12
arpitsardhana
pushed a commit
that referenced
this pull request
Jul 9, 2026
* feat(files): Add OTLP to new nemoclient endpoints Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * lint Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * remove these Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * feat(files): Migrate consumers to the new FilesClient Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * code review Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fixes Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fixes Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fixes Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * lint fix Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(tests): patch client_from_platform in tests that mock SDK internals Tests that mock the NeMoPlatform SDK need to account for client_from_platform accessing _custom_headers and _client.headers. Patch the adapter function in test_schema, test_jobs, and test_models_api to return mock FilesClient instances. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * refactor: rename Files*Error aliases to Client*Error These error aliases from nemo_platform_plugin.client.errors are not files-specific — they come from the generic typed HTTP client. Rename to Client* to reflect that. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> --------- Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
matthewgrossman
added a commit
that referenced
this pull request
Jul 13, 2026
…o-nemoclient-typed-http-client Brings in the merged NemoClient migrations from main: - #584 (Files consumers -> FilesClient) - #609 (Secrets service + consumers -> SecretsClient) - #614 (plugin-client URL encoding) Resolved 4 conflicts where the Jobs consumer migration (AIRCORE-874) overlapped the Files/Secrets migrations in multi-service files: - services/core/entities/.../workspace_cleanup.py: combined both migrations (jobs via AsyncJobsClient, filesets via AsyncFilesClient); switched PlatformJobStatus to the plugin type and built the terminal-status set from enum members. - services/core/entities/tests/.../test_workspace_cleanup.py: adopted #584's tuple-return _make_sdk + _MockAsyncPaginatedResponse; added a dispatching _patch_clients() so the combined _async_step test routes client_from_platform to the jobs vs files mock by client class. - plugins/nemo-safe-synthesizer/.../api/v2/jobs/endpoints.py test: dropped the now-orphaned Stainless NotFoundError/PermissionDeniedError import (both uses migrated across the two PRs); made _patch_jobs_client dispatch jobs vs files clients by class. - plugins/nemo-data-designer/.../testing/utils.py: kept both the jobs (CreatePlatformJobRequest) and secrets (SecretsClient) imports. All affected suites green; jobs service unchanged (5 pre-existing env failures). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Matthew Grossman <mgrossman@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
2 participants
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
Migrates Files consumers from the Stainless SDK's
FilesetsSubResource/AsyncFilesetsSubResourceto the new typedFilesClient/AsyncFilesClientHTTP clients (92 files, +2232/-2293).What changed
client_from_platform(sdk, FilesClient)(orAsyncFilesClient) instead ofsdk.files.filesets.*FilesetFileSystem: Constructor changed fromsdk=kwarg toclient=kwarg, acceptingFilesClientorAsyncFilesClientdirectlyFilesResource: Accepts optionalfiles_client=kwarg for Stainless-agnostic construction;sdk.filesstill works as backward-compatible sugar (delegates toFilesClientinternally)nemo_platform_plugin.client.errorsinstead ofnemo_platformCreateFilesetRequest,UpdateFilesetRequest,FilesetOutput,ListFilesetsQueryParamsetc. fromnemo_platform_plugin.files.typeslist_filesets()returnsNemoPaginatedResponsewith.items()/.pages()/.page()methods; non-paginated responses unwrapped with.data()with_options(timeout=float, retry=RetryPolicy(...))replaceshttpx.Timeoutand integer retry countsnemo files filesets create/list/get/update/deleteall migrated toFilesClient. Thelistcommand now usesListFilesetsQueryParamsandNemoPaginatedResponsedirectly instead of the Stainlessfetch_all_pageshelperDeleted infrastructure
FilesetsSubResource/AsyncFilesetsSubResource_RemappingFilesClient/_RemappingAsyncFilesClient_raw_clienton Files resourcesWhat still uses
sdk.files~30 production files still call through
sdk.files.*(which now delegates toFilesClientinternally). These are future migration targets. Non-files Stainless error imports (entities, models, jobs) remain intentionally.Test plan
make test-unitpassesuv run --frozen pytest packages/nmp_common/tests/jobs/test_file_manager.py— 22 passeduv run --frozen pytest packages/nemo_platform_ext/tests/cli/integration/test_filesets.py— 10 passeduv run --frozen pytest plugins/nemo-evaluator/tests/passesuv run --frozen pytest plugins/nemo-data-designer/tests/passesnemo files filesets create,nemo files filesets listwork correctly against running platform🤖 Generated with Claude Code