Skip to content

refactor(secrets): migrate secrets consumers to typed SecretsClient - #1493

Merged
maxdubrinsky merged 1 commit into
mainfrom
aircore-827-migrate-secrets/mdubrinsky
Aug 27, 2026
Merged

refactor(secrets): migrate secrets consumers to typed SecretsClient#1493
maxdubrinsky merged 1 commit into
mainfrom
aircore-827-migrate-secrets/mdubrinsky

Conversation

@maxdubrinsky

@maxdubrinsky maxdubrinsky commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Migrate secrets API call sites from sdk.secrets.* (Stainless SDK) to client_from_platform(sdk, TypedClient).* (typed HTTP client), following the pattern established in #1277.

Related Issue

AIRCORE-827

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with documentation updates
  • Documentation only
  • Contributor tooling or automation
  • CI, build, or test infrastructure

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification: same API calls, different client
  • Tests not applicable — justification:
  • Documentation updated for user-visible behavior
  • Documentation not applicable — justification: internal refactoring

Verification

  • Pull request title follows the repository's Conventional Commit format
  • Every commit includes an appropriate Signed-off-by: trailer
  • uv run pre-commit run -a passes, or any blocked checks are identified below
  • Targeted tests pass, or tests are marked not applicable above
  • No secrets, API keys, or credentials are included

Summary by CodeRabbit

  • Refactor
    • Standardized secret management across credential, job, and storage workflows.
    • Improved consistency for creating, retrieving, listing, and deleting secrets.
  • Tests
    • Updated end-to-end and unit coverage for secret handling and storage backend scenarios while preserving expected behavior.

@maxdubrinsky
maxdubrinsky force-pushed the aircore-827-migrate-secrets/mdubrinsky branch 3 times, most recently from c4e592e to 9239da5 Compare August 24, 2026 20:51
@maxdubrinsky
maxdubrinsky marked this pull request as ready for review August 24, 2026 21:11
@maxdubrinsky
maxdubrinsky requested review from a team as code owners August 24, 2026 21:11
@maxdubrinsky
maxdubrinsky force-pushed the aircore-827-migrate-secrets/mdubrinsky branch from 9239da5 to 671ccf1 Compare August 24, 2026 21:38
@maxdubrinsky
maxdubrinsky force-pushed the mdubrinsky/aircore-1058-add-typed-clients-for-remaining-service-areas-nemoclient branch from 2070a49 to c8aeb8a Compare August 25, 2026 16:35
Base automatically changed from mdubrinsky/aircore-1058-add-typed-clients-for-remaining-service-areas-nemoclient to main August 25, 2026 20:56
@maxdubrinsky
maxdubrinsky force-pushed the aircore-827-migrate-secrets/mdubrinsky branch from 671ccf1 to 8dcedc5 Compare August 26, 2026 21:30
@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d7d276f2-ffdb-4c0d-85cd-70be192e721e

📥 Commits

Reviewing files that changed from the base of the PR and between 5c6285c and b988e5b.

📒 Files selected for processing (1)
  • e2e/test_jobs.py

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The change replaces direct SDK secret and job operations with platform-adapted clients. E2E tests use typed request models and response data extraction. Nemo Iron Swarm secret resolution uses the platform SecretsClient.

Changes

Secrets client migration

Layer / File(s) Summary
E2E secret fixtures and storage flows
e2e/conftest.py, e2e/files/test_storage_backends.py
Fixtures and storage backend tests create and delete secrets through SecretsClient with PlatformSecretCreateRequest.
E2E secrets API tests
e2e/test_secrets.py
Secret creation, retrieval, listing, deletion, duplicate checks, and secrecy checks now use SecretsClient.
E2E job client flows
e2e/test_jobs.py
Job creation, log retrieval, status checks, cancellation, pause, and resume now use JobsClient. Secret lifecycle checks use SecretsClient.
Plugin secret resolution
plugins/nemo-iron-swarm/src/nemo_iron_swarm_plugin/cli/credentials.py, plugins/nemo-iron-swarm/src/nemo_iron_swarm_plugin/jobs/_common.py
Credential and job secret resolution now use platform-adapted SecretsClient.access_secret(...).data() calls.
Plugin resolution tests
plugins/nemo-iron-swarm/tests/unit/test_model_config.py, plugins/nemo-iron-swarm/tests/unit/test_operator_env.py
Unit-test doubles and fixtures now route secret access through SecretsClient.access_secret and nested data() responses.

Suggested reviewers: a2bondar

Merge Risk: ⚪ Minimal · up to b988e

This refactoring migrates secrets API call sites to a typed client without any supplied evidence of a concrete merge-blocking risk; no actionable risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 27 functions across 8 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: migrating secrets consumers to the typed SecretsClient.
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch aircore-827-migrate-secrets/mdubrinsky

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

@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 35433/45223 78.3% 62.9%
Integration Tests 21119/42998 49.1% 22.3%

@maxdubrinsky
maxdubrinsky force-pushed the aircore-827-migrate-secrets/mdubrinsky branch 2 times, most recently from 18eb543 to 5c6285c Compare August 26, 2026 22:24
@maxdubrinsky
maxdubrinsky added this pull request to the merge queue Aug 26, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Aug 26, 2026
Migrate secrets API call sites from sdk.secrets.* (Stainless SDK)
to client_from_platform(sdk, SecretsClient).* (typed HTTP client),
following the pattern established in #1277.

Changes:
- e2e/conftest.py: secrets.create/delete -> create_secret/delete_secret
  with PlatformSecretCreateRequest body + .data()
- e2e/files/test_storage_backends.py: secrets.create/delete ->
  create_secret/delete_secret with body + .data()
- e2e/test_jobs.py: secrets.create/delete/retrieve ->
  create_secret/delete_secret/get_secret with body + .data()
- e2e/test_secrets.py: secrets.create/list/retrieve/delete ->
  create_secret/list_secrets/get_secret/delete_secret
- plugins/nemo-iron-swarm/credentials.py: secrets.access ->
  access_secret with .data()
- plugins/nemo-iron-swarm/_common.py: secrets.access ->
  access_secret with .data()

The auto-generated CLI file is left for a CLI generator update.

AIRCORE-827

Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>
@maxdubrinsky
maxdubrinsky force-pushed the aircore-827-migrate-secrets/mdubrinsky branch from 5c6285c to b988e5b Compare August 27, 2026 15:11
@maxdubrinsky
maxdubrinsky added this pull request to the merge queue Aug 27, 2026
Merged via the queue into main with commit 4e50273 Aug 27, 2026
58 checks passed
@maxdubrinsky
maxdubrinsky deleted the aircore-827-migrate-secrets/mdubrinsky branch August 27, 2026 16:04
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