refactor: Migrate Infiniband Partition API handlers to WithTx#497
Merged
Conversation
Apply `WithTx` from NVIDIA#462 to the Infiniband Partiton handlers, ensuring the split assign/condition is [re]factored correctly, as well as using the workflow termination helper! Coderabbit feedback addressed locally: - Propagates `StatusDetail` creation failure as an API error in the Delete handler (which matches the Create handler) instead of just logging. Signed-off-by: Chet Nichols III <chetn@nvidia.com>
Contributor
WalkthroughThe InfiniBandPartition handlers (Create, Update, Delete) are refactored from direct database transactions to a unified workflow-driven, transaction-wrapped pattern. Each handler now uses cdb.WithTx closures, introduces StatusDetail DAO for tracking, orchestrates Temporal site workflows, and manages timeouts through deferred termination logic. ChangesWorkflow-Driven Handler Refactor
Sequence DiagramsequenceDiagram
participant Handler as CreateInfiniBandPartition<br/>Handler
participant TxMgr as Transaction<br/>Manager (WithTx)
participant DAO as InfiniBandPartition<br/>& StatusDetail DAO
participant Temporal as Temporal<br/>Workflow Engine
participant Site as Site<br/>Service
Handler->>TxMgr: Begin transaction scope
TxMgr->>DAO: Insert InfiniBandPartition record
DAO-->>TxMgr: Record created with ID
TxMgr->>DAO: Insert Status Detail (Pending)
DAO-->>TxMgr: Status tracked
TxMgr->>Temporal: Get site client
Temporal-->>TxMgr: Client ready
TxMgr->>Temporal: Start CreateInfiniBandPartitionV2 workflow
Temporal-->>TxMgr: Workflow ID, awaiting completion
TxMgr->>Temporal: Wait for workflow result (with timeout)
par Workflow Execution
Temporal->>Site: Execute provisioning operations
Site-->>Temporal: Provisioning complete
end
alt Workflow Completes
Temporal-->>TxMgr: Success result
TxMgr->>DAO: Collect final Status Details
DAO-->>TxMgr: Status details retrieved
TxMgr-->>TxMgr: Commit transaction
TxMgr-->>Handler: Success with created entity & status
Handler-->>Handler: Return 201 Created
else Timeout Occurs
Temporal-->>TxMgr: Timeout
TxMgr-->>TxMgr: Commit & prepare termination
TxMgr->>Temporal: Schedule TerminateWorkflow (deferred)
TxMgr-->>Handler: Accepted (202)
Handler-->>Handler: Return 202 Accepted
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-05-06 20:41:16 UTC | Commit: 90cc9ad |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
api/pkg/api/handler/infinibandpartition.go (2)
244-245: ⚡ Quick winRedundant pointer allocation and dereference.
*cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusPending)allocates a string pointer on the heap only to immediately dereference it. Use the constant directly, consistent with the established pattern in other handlers (e.g.,vpc.go).♻️ Proposed simplification
- newSSD, derr := sdDAO.CreateFromParams(ctx, tx, ibp.ID.String(), *cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusPending), + newSSD, derr := sdDAO.CreateFromParams(ctx, tx, ibp.ID.String(), cdbm.InfiniBandPartitionStatusPending, cdb.GetStrPtr("received InfiniBand Partition creation request, pending"))🤖 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 `@api/pkg/api/handler/infinibandpartition.go` around lines 244 - 245, Replace the redundant allocate-and-dereference pattern used for the status argument in the sdDAO.CreateFromParams call: remove the unnecessary *cdb.GetStrPtr(...) and pass the constant cdbm.InfiniBandPartitionStatusPending directly (keep the other parameter using cdb.GetStrPtr("received InfiniBand Partition creation request, pending") unchanged); update the call that assigns newSSD from sdDAO.CreateFromParams so it uses cdbm.InfiniBandPartitionStatusPending instead of *cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusPending).
1050-1055: ⚡ Quick winSame redundant pointer pattern as Create handler.
Apply the same simplification here for consistency.
♻️ Proposed simplification
// Create status detail - if _, derr := sdDAO.CreateFromParams(ctx, tx, ibp.ID.String(), *cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusDeleting), + if _, derr := sdDAO.CreateFromParams(ctx, tx, ibp.ID.String(), cdbm.InfiniBandPartitionStatusDeleting, cdb.GetStrPtr("Received request for deletion, pending processing")); derr != nil {🤖 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 `@api/pkg/api/handler/infinibandpartition.go` around lines 1050 - 1055, The inline cdb.GetStrPtr calls used when creating the Status Detail are redundant; mirror the Create handler simplification by creating local variables (e.g., statusPtr and msgPtr) using cdb.GetStrPtr for cdbm.InfiniBandPartitionStatusDeleting and the deletion message, then pass those locals to sdDAO.CreateFromParams instead of calling cdb.GetStrPtr inline; update the call around sdDAO.CreateFromParams, keep the same error handling (logger.Error().Err(derr).Msg...) and return cutil.NewAPIError as before.
🤖 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.
Nitpick comments:
In `@api/pkg/api/handler/infinibandpartition.go`:
- Around line 244-245: Replace the redundant allocate-and-dereference pattern
used for the status argument in the sdDAO.CreateFromParams call: remove the
unnecessary *cdb.GetStrPtr(...) and pass the constant
cdbm.InfiniBandPartitionStatusPending directly (keep the other parameter using
cdb.GetStrPtr("received InfiniBand Partition creation request, pending")
unchanged); update the call that assigns newSSD from sdDAO.CreateFromParams so
it uses cdbm.InfiniBandPartitionStatusPending instead of
*cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusPending).
- Around line 1050-1055: The inline cdb.GetStrPtr calls used when creating the
Status Detail are redundant; mirror the Create handler simplification by
creating local variables (e.g., statusPtr and msgPtr) using cdb.GetStrPtr for
cdbm.InfiniBandPartitionStatusDeleting and the deletion message, then pass those
locals to sdDAO.CreateFromParams instead of calling cdb.GetStrPtr inline; update
the call around sdDAO.CreateFromParams, keep the same error handling
(logger.Error().Err(derr).Msg...) and return cutil.NewAPIError as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 265be689-7865-42cd-9317-0a1076d1f583
📒 Files selected for processing (1)
api/pkg/api/handler/infinibandpartition.go
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
thossain-nv
approved these changes
May 7, 2026
Contributor
thossain-nv
left a comment
thossain-nv
left a comment
There was a problem hiding this comment.
Looks good, thank you @chet
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.
Description
Apply
WithTxfrom #462 to the Infiniband Partiton handlers, ensuring the split assign/condition is [re]factored correctly, as well as using the workflow termination helper (TerminateWorkflowOnTimeOut).Coderabbit feedback addressed locally:
StatusDetailcreation failure as an API error in the Delete handler (which matches the Create handler) instead of just logging.Signed-off-by: Chet Nichols III chetn@nvidia.com
Type of Change
Services Affected
Related Issues (Optional)
Breaking Changes
Testing
Additional Notes