Skip to content

feat(files): Add OTLP to new nemoclient endpoints - #576

Merged
matthewgrossman merged 3 commits into
mainfrom
mgrossman/aircore-840-migrate-files-consumers-from-filesetssubresource-to
Jul 6, 2026
Merged

feat(files): Add OTLP to new nemoclient endpoints#576
matthewgrossman merged 3 commits into
mainfrom
mgrossman/aircore-840-migrate-files-consumers-from-filesetssubresource-to

Conversation

@matthewgrossman

@matthewgrossman matthewgrossman commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Part 1 of AIRCORE-840. Migrates OTLP log operations from the Stainless SDK resource chain (sdk.files.otlp.logs.*) to typed NemoClient endpoints, and removes the _raw_client / .otlp shims introduced in #429.

  • Add upload_otlp_logs and query_otlp_logs typed endpoints + types to nemo_platform_plugin.files
  • Rewrite JobLogsClient to use AsyncFilesClient.query_otlp_logs instead of sdk.files.otlp.logs.query
  • Rewrite CLI nemo files otlp logs create/query commands to use FilesClient via client_from_platform
  • Remove _raw_client and .otlp property from FilesResource / AsyncFilesResource (both packages/filesets and vendored SDK)
  • Update test_log_client.py to mock AsyncFilesClient instead of Stainless SDK, use plugin NotFoundError

Test plan

  • packages/nmp_common/tests/jobs/test_log_client.py — 9/9 pass
  • services/core/jobs/tests/test_job_logs.py — 7/7 pass (API-level tests)
  • e2e/test_jobs.py --run-e2e — 10/10 pass, including test_job_logs_across_multiple_batches which exercises the full OTLP pipeline end-to-end through JobLogsClientAsyncFilesClient.query_otlp_logs → files service
  • Ruff lint clean
  • Vendored SDK copy in sync with packages/filesets

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added OTLP log upload and query support through the Files API.
    • Introduced structured request and response handling for OTLP log operations.
  • Bug Fixes

    • Updated job log retrieval to use the new typed Files API path, improving consistency and pagination handling.
    • Empty filter requests now default correctly, and missing log results return an empty page as expected.

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
@matthewgrossman
matthewgrossman requested review from a team as code owners July 6, 2026 17:56
@github-actions github-actions Bot added the feat label Jul 6, 2026
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Removes the Stainless SDK's otlp cached-property delegation from FilesResource/AsyncFilesResource. Adds typed upload_otlp_logs/query_otlp_logs endpoints, DTOs, and client wiring in nemo_platform_plugin. Migrates JobLogsClient to call the typed AsyncFilesClient instead of the SDK, with corresponding test updates.

Changes

OTLP Logs Client Migration

Layer / File(s) Summary
OTLP request/response types and endpoint contracts
packages/nemo_platform_plugin/src/nemo_platform_plugin/files/types.py, .../files/endpoints.py, .../files/client.py
Adds OtlpLogQueryRequest, OtlpExportLogsPartialSuccess, OtlpExportLogsResponse DTOs; new upload_otlp_logs/query_otlp_logs POST endpoints; wires both onto FilesClient/AsyncFilesClient.
Remove Stainless SDK otlp resources
packages/filesets/src/filesets/resources.py
Drops _raw_client assignment and otlp cached-property from sync/async FilesResource classes.
JobLogsClient uses typed FilesClient
packages/nmp_common/src/nmp/common/jobs/log_client.py, packages/nmp_common/tests/jobs/test_log_client.py
JobLogsClient now builds an AsyncFilesClient via client_from_platform and queries logs with OtlpLogQueryRequest, returning resp.data(); tests updated to mock the typed client instead of the SDK.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant JobLogsClient
  participant AsyncFilesClient
  Caller->>JobLogsClient: query_logs(filters, limit, page_cursor)
  JobLogsClient->>AsyncFilesClient: query_otlp_logs(name, workspace, body)
  AsyncFilesClient-->>JobLogsClient: resp
  JobLogsClient-->>Caller: resp.data()
Loading

Suggested reviewers: maxdubrinsky, mckornfield

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main change: adding OTLP file endpoints and migrating consumers to them.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mgrossman/aircore-840-migrate-files-consumers-from-filesetssubresource-to

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@packages/nemo_platform_plugin/src/nemo_platform_plugin/files/endpoints.py`:
- Around line 95-97: The OTLP logs upload path is missing the request payload,
so the parsed log content is never sent. Update the `upload_otlp_logs` abstract
method in `endpoints.py` to accept a payload/body argument, then thread that
value through the CLI command in
`nemo_platform_ext/cli/commands/api/files/otlp/logs.py` so the parsed
`--input-file/--input-data` content is passed into the upload call. Make sure
the implementation and interface stay aligned for `upload_otlp_logs` and the
related request-building code.
🪄 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: f5d9eea2-96db-4be4-8ada-9c62efd2d429

📥 Commits

Reviewing files that changed from the base of the PR and between fb50b86 and 96e40d4.

⛔ Files ignored due to path filters (1)
  • sdk/python/nemo-platform/src/nemo_platform/filesets/resources.py is excluded by !sdk/**
📒 Files selected for processing (7)
  • packages/filesets/src/filesets/resources.py
  • packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/files/otlp/logs.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/files/client.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/files/endpoints.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/files/types.py
  • packages/nmp_common/src/nmp/common/jobs/log_client.py
  • packages/nmp_common/tests/jobs/test_log_client.py
💤 Files with no reviewable changes (1)
  • packages/filesets/src/filesets/resources.py

Comment thread packages/nemo_platform_plugin/src/nemo_platform_plugin/files/endpoints.py Outdated
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 23242/30406 76.4% 61.2%
Integration Tests 13588/29086 46.7% 19.9%

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@packages/nemo_platform_plugin/src/nemo_platform_plugin/files/endpoints.py`:
- Around line 97-99: The shared upload signature still allows
AsyncIterable[bytes], but FilesClient passes request.content into
httpx.Client.request(), which only supports bytes or sync iterables. Update
upload_otlp_logs to remove AsyncIterable[bytes] from this shared endpoint
signature, and keep async streaming separate if needed by a different path or
overload. Make sure the types used by upload_otlp_logs and any callers in the
FilesClient flow match the actual httpx.Client.request() contract.
🪄 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: 6e666ead-ea62-4161-9b07-cd52abe7cf86

📥 Commits

Reviewing files that changed from the base of the PR and between 96e40d4 and 9e2049c.

📒 Files selected for processing (1)
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/files/endpoints.py

@matthewgrossman
matthewgrossman added this pull request to the merge queue Jul 6, 2026
Merged via the queue into main with commit 2983dc4 Jul 6, 2026
55 checks passed
@matthewgrossman
matthewgrossman deleted the mgrossman/aircore-840-migrate-files-consumers-from-filesetssubresource-to branch July 6, 2026 20:39
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>

---------

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants