Skip to content

refactor: Migrate instance handler to WithTx#476

Merged
chet merged 2 commits intoNVIDIA:mainfrom
chet:with-tx-instance
May 4, 2026
Merged

refactor: Migrate instance handler to WithTx#476
chet merged 2 commits intoNVIDIA:mainfrom
chet:with-tx-instance

Conversation

@chet
Copy link
Copy Markdown
Contributor

@chet chet commented May 4, 2026

Description

Applies the WithTx pattern from #462 to 4 sites in instance.go: Create, handleReboot, Update, and Delete.

This required a few mechanical adjustments:

  • Workflow execution + timeout handling moved inside each closure; inner errors renamed to "wferr" so they don't shadow the outer "err".
  • Result variables (instance, ifcs, ui, newdbIfcs, ...) are now declared outside the closures and assigned inside, since the response builders need them after the tx commits (but this is to be expected).
  • buildInstanceUpdateRequestOsConfig used to write the HTTP response directly via c.JSON(...) inside the tx. The helper now returns a *cutil.APIError that flows through HandleTxError like everything else.

Keeping these PRs smaller and tightly scoped so they're:

  • In theory a little easier to read.
  • More tightly scoped / less "blast radius" per PR.
  • A little nicer on/for @coderabbitai. 😆

I do wish the diffs were easier to read, but it is what it is!

Signed-off-by: Chet Nichols III chetn@nvidia.com

Type of Change

  • Feature - New feature or functionality (feat:)
  • Fix - Bug fixes (fix:)
  • Chore - Modification or removal of existing functionality (chore:)
  • Refactor - Refactoring of existing functionality (refactor:)
  • Docs - Changes in documentation or OpenAPI schema (docs:)
  • CI - Changes in GitHub workflows. Requires additional scrutiny (ci:)
  • Version - Issuing a new release version (version:)

Services Affected

  • API - API models or endpoints updated
  • Workflow - Workflow service updated
  • DB - DB DAOs or migrations updated
  • Site Manager - Site Manager updated
  • Cert Manager - Cert Manager updated
  • Site Agent - Site Agent updated
  • RLA - RLA service updated
  • Powershelf Manager - Powershelf Manager updated
  • NVSwitch Manager - NVSwitch Manager updated

Related Issues (Optional)

Breaking Changes

  • This PR contains breaking changes

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • No testing required (docs, internal refactor, etc.)

Additional Notes

Applies the `WithTx` pattern from NVIDIA#462 to 4 sites in instance.go: `Create`, `handleReboot`, `Update`, and `Delete`.

This required a few mechanical adjustments:
- Workflow execution + timeout handling moved inside each closure; inner errors renamed to `"wferr"` so they don't shadow the outer `"err"`.
- Result variables (`instance`, `ifcs`, `ui`, `newdbIfcs`, ...) are now declared outside the closures and assigned inside, since the response builders need them after the tx commits (but this is to be expected).
- `buildInstanceUpdateRequestOsConfig` used to write the HTTP response directly via `c.JSON(...)` inside the `tx`. The helper now returns a `*cutil.APIError` that flows through `HandleTxError` like everything else.

Keeping these PRs smaller and tightly scoped so they're:
- In theory a little easier to read.
- More tightly scoped / less "blast radius" per PR.
- A little nicer on/for @coderabbitai. 😆

I do wish the diffs were easier to read, but it is what it is!

Signed-off-by: Chet Nichols III <chetn@nvidia.com>
@chet chet requested a review from a team as a code owner May 4, 2026 15:53
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 4, 2026

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

💤 Files selected but had no reviewable changes (1)
  • api/pkg/api/handler/instance.go
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 52c67578-ea42-4902-82ef-89a0d957f655

📥 Commits

Reviewing files that changed from the base of the PR and between e02c20b and 01d31ef.

📒 Files selected for processing (1)
  • api/pkg/api/handler/instance.go

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Review rate limit: 9/10 reviews remaining, refill in 6 minutes.

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

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 4, 2026

🔐 TruffleHog Secret Scan

No secrets or credentials found!

Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉

🔗 View scan details

🕐 Last updated: 2026-05-04 15:55:29 UTC | Commit: 2755e99

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 4, 2026

🔍 Container Scan Summary

Service Total Critical High Medium Low Other
carbide-nsm 66 4 20 33 9 0
carbide-psm 58 6 29 13 2 8
carbide-rest-api 60 6 31 13 2 8
carbide-rest-cert-manager 54 4 28 13 1 8
carbide-rest-db 58 6 29 13 2 8
carbide-rest-site-agent 55 5 28 13 1 8
carbide-rest-site-manager 54 4 28 13 1 8
carbide-rest-workflow 59 6 30 13 2 8
carbide-rla 57 6 28 13 2 8
TOTAL 521 47 251 137 22 64

Per-CVE detail lives in the per-service grype-* artifacts (JSON + SARIF). Severity counts only — no CVE IDs published here.

Copy link
Copy Markdown
Contributor

@pbreton pbreton left a comment

Choose a reason for hiding this comment

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

LGTM.

@chet chet merged commit 3e51a84 into NVIDIA:main May 4, 2026
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants