change(rest-api): Align REST API with new VpcPrefix lifecycle#2272
change(rest-api): Align REST API with new VpcPrefix lifecycle#2272bcavnvidia wants to merge 1 commit into
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rest-api/openapi/spec.yaml (1)
14351-14358: ⚡ Quick winConsider enhancing state documentation for API consumers.
The enum values are correct, but the generic description "Status values for VPC Prefix objects" could be more informative. Production APIs benefit from explicit state semantics to reduce integration friction.
Consider documenting:
- What each state represents (e.g., "Provisioning: Initial state during prefix allocation")
- Valid state transitions (e.g., "Provisioning → Ready → Deleting → Deleted")
- Any terminal states (Error, Deleted)
This helps API consumers understand the lifecycle without consulting additional documentation.
📝 Enhanced documentation example
VpcPrefixStatus: title: VpcPrefixStatus type: string - description: Status values for VPC Prefix objects + description: | + Status of VPC Prefix lifecycle: + - Provisioning: Prefix is being allocated + - Ready: Prefix is active and available + - Deleting: Prefix deletion in progress + - Deleted: Prefix has been removed + - Error: Prefix encountered an error during lifecycle operation enum: - Provisioning - Ready - Deleting - Deleted - Error
🤖 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 `@rest-api/workflow/pkg/activity/vpcprefix/vpcprefix.go`:
- Around line 121-123: The current guard in vpcprefix.go that checks "if
vpcPrefix.Status == cdbm.VpcPrefixStatusDeleting && status !=
cdbm.VpcPrefixStatusDeleting { continue }" prevents persisting a transition from
Deleting to Deleted; change the condition to allow the terminal transition by
only skipping updates when the existing status is Deleting and the new status is
neither Deleting nor Deleted (i.e., also check cdbm.VpcPrefixStatusDeleted), so
the Deleting->Deleted transition is permitted; update the condition that
references vpcPrefix.Status and the local variable status accordingly.
🪄 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: 74b06d0f-f73a-449a-beb7-b7e8b1b37102
⛔ Files ignored due to path filters (3)
rest-api/workflow-schema/schema/site-agent/workflows/v1/nico_nico.pb.gois excluded by!**/*.pb.go,!rest-api/**/*.pb.gorest-api/workflow-schema/schema/site-agent/workflows/v1/nico_nico_grpc.pb.gois excluded by!**/*.pb.go,!rest-api/**/*.pb.gorest-api/workflow-schema/site-agent/workflows/v1/nico_nico.protois excluded by!rest-api/workflow-schema/site-agent/workflows/v1/*_nico.proto
📒 Files selected for processing (14)
rest-api/api/pkg/api/handler/vpcprefix.gorest-api/api/pkg/api/handler/vpcprefix_test.gorest-api/db/pkg/db/model/vpcprefix.gorest-api/docs/index.htmlrest-api/openapi/spec.yamlrest-api/sdk/standard/model_interface.gorest-api/sdk/standard/model_interface_create_request.gorest-api/sdk/standard/model_tenant_identity_config_create_or_update_request.gorest-api/sdk/standard/model_vpc_prefix_status.gorest-api/site-workflow/pkg/activity/vpcprefix.gorest-api/site-workflow/pkg/activity/vpcprefix_test.gorest-api/site-workflow/pkg/grpc/client/testing.gorest-api/workflow/pkg/activity/vpcprefix/vpcprefix.gorest-api/workflow/pkg/activity/vpcprefix/vpcprefix_test.go
💤 Files with no reviewable changes (1)
- rest-api/sdk/standard/model_tenant_identity_config_create_or_update_request.go
| if vpcPrefix.Status == cdbm.VpcPrefixStatusDeleting && status != cdbm.VpcPrefixStatusDeleting { | ||
| continue | ||
| } |
There was a problem hiding this comment.
Allow Deleting -> Deleted transition instead of skipping all non-deleting updates.
This guard prevents updating a prefix from Deleting to Deleted, so terminal lifecycle state cannot be persisted for already-deleting records.
💡 Proposed fix
- if vpcPrefix.Status == cdbm.VpcPrefixStatusDeleting && status != cdbm.VpcPrefixStatusDeleting {
+ if vpcPrefix.Status == cdbm.VpcPrefixStatusDeleting &&
+ status != cdbm.VpcPrefixStatusDeleting &&
+ status != cdbm.VpcPrefixStatusDeleted {
continue
}🤖 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 `@rest-api/workflow/pkg/activity/vpcprefix/vpcprefix.go` around lines 121 -
123, The current guard in vpcprefix.go that checks "if vpcPrefix.Status ==
cdbm.VpcPrefixStatusDeleting && status != cdbm.VpcPrefixStatusDeleting {
continue }" prevents persisting a transition from Deleting to Deleted; change
the condition to allow the terminal transition by only skipping updates when the
existing status is Deleting and the new status is neither Deleting nor Deleted
(i.e., also check cdbm.VpcPrefixStatusDeleted), so the Deleting->Deleted
transition is permitted; update the condition that references vpcPrefix.Status
and the local variable status accordingly.
1 participant
Description
VpcPrefix in the Rust back-end now has a full lifecycle. The REST API still only expects Ready/Deleting. This PR aligns the two, which mostly means a Provisioning state.
Type of Change
Related Issues (Optional)
Breaking Changes
Testing
Additional Notes