feat: update DB endpoints to match current spec#635
Conversation
✅ Deploy Preview for open-api ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis pull request refactors database branch management from a deploy-scoped to a branch-scoped model by systematically replacing Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 4
🧹 Nitpick comments (2)
go/models/database_compute_settings.go (1)
18-30: Consider adding contract-level bounds for compute settings.
Validateis currently a no-op, so negative/invalid numeric combinations won’t be caught client-side. If these fields have domain constraints, encode them in the OpenAPI schema so generated validation enforces them automatically.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go/models/database_compute_settings.go` around lines 18 - 30, The Validate method on DatabaseComputeSettings is a no-op so invalid values slip through; update DatabaseComputeSettings.Validate to check that MaxCu and MinCu are non-negative, that MinCu <= MaxCu when both are set, and that SleepTimeoutSeconds is non-negative (and enforce any additional domain rules you expect), returning a descriptive error on violation; also add corresponding constraints to the OpenAPI/schema for MaxCu, MinCu, and SleepTimeoutSeconds so generated validation will catch these rules in clients and server stubs.go/plumbing/operations/delete_site_database_branch_parameters.go (1)
110-119: Add explicit SDK migration notes for renamed fluent methods.Removing
WithDeployID/SetDeployIDis a source-level client break; please call this out in changelog/release notes with a concise before/after example.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go/plumbing/operations/delete_site_database_branch_parameters.go` around lines 110 - 119, The public API removed WithDeployID/SetDeployID and introduced WithBranchID/SetBranchID (methods on DeleteSiteDatabaseBranchParams), which is a source-breaking change; update the release notes/changelog to include an explicit SDK migration note that documents the rename with a concise before/after example showing callers replacing WithDeployID(...) / SetDeployID(...) with WithBranchID(...) / SetBranchID(...), and mention the impacted type DeleteSiteDatabaseBranchParams so users can find and update usages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go/plumbing/operations/operations_client.go`:
- Line 42: The added method ClearSiteDatabaseComputeSettings on the exported
ClientService interface is a source-breaking change for downstream
implementations; update consumer-facing artifacts by (a) documenting this as a
breaking change in the release notes/CHANGELOG referencing ClientService and the
new ClearSiteDatabaseComputeSettings method, and (b) bumping the module major
version (or otherwise following your semver policy) so downstream users know to
update their implementations; alternatively consider providing a compatibility
shim or helper stub for ClientService to ease migration for existing custom
implementations.
In
`@go/plumbing/operations/set_site_database_branch_compute_settings_parameters.go`:
- Around line 160-163: The generated client currently doesn't enforce the CU
bounds/cross-field invariants because DatabaseComputeSettingsRequest.Validate()
is a no-op; update the API/swagger schema for DatabaseComputeSettingsRequest to
encode the documented constraints (min/max for CU fields and any cross-field
rules such as lower <= upper or mutual exclusivity) so the code generator will
emit proper validation, then re-generate the client so calls that set
o.ComputeSettings (used in SetBodyParam) will be validated locally via the
generated Validate() method before being sent.
In `@go/plumbing/operations/set_site_database_compute_settings_parameters.go`:
- Around line 139-143: In WriteToRequest for the
SetSiteDatabaseComputeSettingsParameters operation (the code block that
currently checks o.ComputeSettings and calls r.SetBodyParam), add a guard that
returns an error when o.ComputeSettings is nil so the client fails fast for
bodyless project-level compute updates; update the function (WriteToRequest) on
the SetSiteDatabaseComputeSettingsParameters type to validate ComputeSettings is
present before calling r.SetBodyParam and return a descriptive error (e.g.,
"ComputeSettings is required") instead of silently sending an empty PUT.
In `@swagger.yml`:
- Around line 3902-3927: The schema for databaseComputeSettingsRequest currently
uses non-nullable numeric types so the generated Go client cannot distinguish
"unset" from zero and the generated Validate() is a no-op; update the OpenAPI
schema to mark min_cu, max_cu, and sleep_timeout_seconds as nullable (e.g., add
nullable: true) and add proper numeric constraints (min_cu and max_cu: minimum
0.25, maximum 16.0; also document/express that max_cu - min_cu must be <= 8.0;
sleep_timeout_seconds: allow -1 or any non-negative integer) so the generator
produces pointer types, and then implement validation logic in the model's
Validate() method (DatabaseComputeSettingsRequest.Validate) to: treat nil
pointers as "not provided", enforce min/max bounds on provided values, ensure
max_cu >= min_cu when both provided, and ensure (max_cu - min_cu) <= 8.0; keep
error messages clear and reference the field names min_cu, max_cu,
sleep_timeout_seconds.
---
Nitpick comments:
In `@go/models/database_compute_settings.go`:
- Around line 18-30: The Validate method on DatabaseComputeSettings is a no-op
so invalid values slip through; update DatabaseComputeSettings.Validate to check
that MaxCu and MinCu are non-negative, that MinCu <= MaxCu when both are set,
and that SleepTimeoutSeconds is non-negative (and enforce any additional domain
rules you expect), returning a descriptive error on violation; also add
corresponding constraints to the OpenAPI/schema for MaxCu, MinCu, and
SleepTimeoutSeconds so generated validation will catch these rules in clients
and server stubs.
In `@go/plumbing/operations/delete_site_database_branch_parameters.go`:
- Around line 110-119: The public API removed WithDeployID/SetDeployID and
introduced WithBranchID/SetBranchID (methods on DeleteSiteDatabaseBranchParams),
which is a source-breaking change; update the release notes/changelog to include
an explicit SDK migration note that documents the rename with a concise
before/after example showing callers replacing WithDeployID(...) /
SetDeployID(...) with WithBranchID(...) / SetBranchID(...), and mention the
impacted type DeleteSiteDatabaseBranchParams so users can find and update
usages.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 38c5beb6-3d83-49b5-ba31-2d48ac0ae969
📒 Files selected for processing (32)
go/models/create_database_branch_request.gogo/models/create_database_snapshot_request.gogo/models/database_branch_compute.gogo/models/database_branch_detail.gogo/models/database_branch_response.gogo/models/database_branches_response.gogo/models/database_compute_settings.gogo/models/database_compute_settings_request.gogo/models/database_snapshot.gogo/models/database_snapshot_metadata.gogo/models/restore_database_snapshot_request.gogo/models/run_database_migrations_request.gogo/plumbing/operations/clear_site_database_compute_settings_parameters.gogo/plumbing/operations/clear_site_database_compute_settings_responses.gogo/plumbing/operations/create_site_database_branch_responses.gogo/plumbing/operations/delete_site_database_branch_parameters.gogo/plumbing/operations/delete_site_database_branch_responses.gogo/plumbing/operations/get_site_database_branch_parameters.gogo/plumbing/operations/get_site_database_branch_responses.gogo/plumbing/operations/get_site_database_compute_settings_parameters.gogo/plumbing/operations/get_site_database_compute_settings_responses.gogo/plumbing/operations/get_site_database_parameters.gogo/plumbing/operations/list_site_database_branches_parameters.gogo/plumbing/operations/list_site_database_branches_responses.gogo/plumbing/operations/operations_client.gogo/plumbing/operations/run_site_database_migrations_parameters.gogo/plumbing/operations/run_site_database_migrations_responses.gogo/plumbing/operations/set_site_database_branch_compute_settings_parameters.gogo/plumbing/operations/set_site_database_branch_compute_settings_responses.gogo/plumbing/operations/set_site_database_compute_settings_parameters.gogo/plumbing/operations/set_site_database_compute_settings_responses.goswagger.yml
|
|
||
| CancelSiteDeploy(params *CancelSiteDeployParams, authInfo runtime.ClientAuthInfoWriter) (*CancelSiteDeployCreated, error) | ||
|
|
||
| ClearSiteDatabaseComputeSettings(params *ClearSiteDatabaseComputeSettingsParams, authInfo runtime.ClientAuthInfoWriter) (*ClearSiteDatabaseComputeSettingsNoContent, error) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Go, is adding a method to an exported interface a source-breaking change for existing downstream implementations?
💡 Result:
Yes. In Go, a concrete type satisfies an interface only if it implements all of the interface’s methods, so adding a new method to an exported interface makes any existing downstream type that implemented the old interface stop compiling (“does not implement … (missing method …)”). This is a source-breaking API change for any users who provide their own implementations/mocks of that interface. [1], [2]
A common mitigation is to design so users don’t implement your exported interface (e.g., return concrete types and let consumers define their own interfaces), or to “seal” the interface by including an unexported method so only your package can implement it—then you can add exported methods later without breaking external implementations (because there are none). [1], [3]
Sources:
[1] Go blog: Keeping Your Modules Compatible
[2] Merovius: Backwards compatibility in Go
[3] apidiff compatibility rules (Go exp tool)
🏁 Script executed:
# Check the file structure and find the ClientService interface definition
cd go/plumbing/operations && head -100 operations_client.go | cat -nRepository: netlify/open-api
Length of output: 6134
🏁 Script executed:
# Search for the ClientService interface definition
rg "type ClientService" go/plumbing/operations/ -A 20Repository: netlify/open-api
Length of output: 2333
🏁 Script executed:
# Check the specific lines mentioned in the review (42, 210, 262, 304-312)
sed -n '40,45p;208,212p;260,265p;302,315p' go/plumbing/operations/operations_client.go | cat -nRepository: netlify/open-api
Length of output: 2476
🏁 Script executed:
# Check if there's an unexported method or sealing pattern in the interface
rg "type ClientService" go/plumbing/operations/operations_client.go -A 300 | head -350 | tail -20Repository: netlify/open-api
Length of output: 1338
🏁 Script executed:
# Check for any version information or generator configuration
fd "swagger\|openapi\|generator" go/plumbing/ 2>/dev/null | head -10Repository: netlify/open-api
Length of output: 42
Note that ClientService interface expansion is a source-breaking change for downstream implementations.
In Go, adding methods to an exported interface requires all existing implementations to add those methods, breaking any downstream custom implementations or mocks. However, since operations_client.go is auto-generated from the OpenAPI spec by go-swagger, this is an inevitable consequence of API evolution. Consider communicating this as a breaking change in release notes or bump the module version accordingly if backward compatibility matters for your consumers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go/plumbing/operations/operations_client.go` at line 42, The added method
ClearSiteDatabaseComputeSettings on the exported ClientService interface is a
source-breaking change for downstream implementations; update consumer-facing
artifacts by (a) documenting this as a breaking change in the release
notes/CHANGELOG referencing ClientService and the new
ClearSiteDatabaseComputeSettings method, and (b) bumping the module major
version (or otherwise following your semver policy) so downstream users know to
update their implementations; alternatively consider providing a compatibility
shim or helper stub for ClientService to ease migration for existing custom
implementations.
| if o.ComputeSettings != nil { | ||
| if err := r.SetBodyParam(o.ComputeSettings); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Encode the CU invariants in the schema before regenerating.
DatabaseComputeSettingsRequest documents bounds and cross-field rules, but its generated Validate() is currently a no-op, so this writer won't reject out-of-range or inverted values locally. If those constraints are part of the contract, add them to the swagger schema so the generated client enforces them too.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@go/plumbing/operations/set_site_database_branch_compute_settings_parameters.go`
around lines 160 - 163, The generated client currently doesn't enforce the CU
bounds/cross-field invariants because DatabaseComputeSettingsRequest.Validate()
is a no-op; update the API/swagger schema for DatabaseComputeSettingsRequest to
encode the documented constraints (min/max for CU fields and any cross-field
rules such as lower <= upper or mutual exclusivity) so the code generator will
emit proper validation, then re-generate the client so calls that set
o.ComputeSettings (used in SetBodyParam) will be validated locally via the
generated Validate() method before being sent.
| if o.ComputeSettings != nil { | ||
| if err := r.SetBodyParam(o.ComputeSettings); err != nil { | ||
| return err | ||
| } | ||
| } |
There was a problem hiding this comment.
Reject bodyless project-level compute updates locally.
WriteToRequest will happily send PUT /sites/{site_id}/database/compute/settings with no payload when ComputeSettings is nil. Since reset already has a dedicated DELETE, this should be modeled as a required body in the swagger spec so the generated client fails fast instead of round-tripping a 4xx.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go/plumbing/operations/set_site_database_compute_settings_parameters.go`
around lines 139 - 143, In WriteToRequest for the
SetSiteDatabaseComputeSettingsParameters operation (the code block that
currently checks o.ComputeSettings and calls r.SetBodyParam), add a guard that
returns an error when o.ComputeSettings is nil so the client fails fast for
bodyless project-level compute updates; update the function (WriteToRequest) on
the SetSiteDatabaseComputeSettingsParameters type to validate ComputeSettings is
present before calling r.SetBodyParam and return a descriptive error (e.g.,
"ComputeSettings is required") instead of silently sending an empty PUT.
| databaseComputeSettingsRequest: | ||
| type: object | ||
| description: >- | ||
| Request body for setting compute settings. | ||
| All fields are optional; only provided fields are updated. | ||
| properties: | ||
| min_cu: | ||
| type: number | ||
| format: double | ||
| description: >- | ||
| Minimum compute units (0.25 to 16.0). | ||
| Must be less than or equal to max_cu. | ||
| max_cu: | ||
| type: number | ||
| format: double | ||
| description: >- | ||
| Maximum compute units (0.25 to 16.0). | ||
| Must be greater than or equal to min_cu. | ||
| max_cu - min_cu must not exceed 8.0. | ||
| sleep_timeout_seconds: | ||
| type: integer | ||
| format: int64 | ||
| description: >- | ||
| Seconds of inactivity before the compute endpoint is suspended. | ||
| Use -1 for always on, or a non-negative value. | ||
| databaseComputeSettings: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Swagger fragment:"
sed -n '3902,3927p' swagger.yml
echo
echo "Generated Go model:"
sed -n '13,35p' go/models/database_compute_settings_request.goRepository: netlify/open-api
Length of output: 1954
Make the compute-settings patch fields nullable and machine-validated.
These properties are documented as optional partial-update fields, but the schema defines them as plain numbers/integers without nullable or validation constraints. The generated Go client uses value types (float64 for min_cu/max_cu, int64 for sleep_timeout_seconds) with omitempty tags, which cannot distinguish between "not provided" and "provided as zero". Additionally, the Validate() method is a no-op that performs no range checking, allowing out-of-range values to be sent.
Recommended schema adjustments
databaseComputeSettingsRequest:
type: object
description: >-
Request body for setting compute settings.
All fields are optional; only provided fields are updated.
properties:
min_cu:
type: number
format: double
+ x-nullable: true
+ minimum: 0.25
+ maximum: 16.0
description: >-
Minimum compute units (0.25 to 16.0).
Must be less than or equal to max_cu.
max_cu:
type: number
format: double
+ x-nullable: true
+ minimum: 0.25
+ maximum: 16.0
description: >-
Maximum compute units (0.25 to 16.0).
Must be greater than or equal to min_cu.
max_cu - min_cu must not exceed 8.0.
sleep_timeout_seconds:
type: integer
format: int64
+ x-nullable: true
+ minimum: -1
description: >-
Seconds of inactivity before the compute endpoint is suspended.
Use -1 for always on, or a non-negative value.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| databaseComputeSettingsRequest: | |
| type: object | |
| description: >- | |
| Request body for setting compute settings. | |
| All fields are optional; only provided fields are updated. | |
| properties: | |
| min_cu: | |
| type: number | |
| format: double | |
| description: >- | |
| Minimum compute units (0.25 to 16.0). | |
| Must be less than or equal to max_cu. | |
| max_cu: | |
| type: number | |
| format: double | |
| description: >- | |
| Maximum compute units (0.25 to 16.0). | |
| Must be greater than or equal to min_cu. | |
| max_cu - min_cu must not exceed 8.0. | |
| sleep_timeout_seconds: | |
| type: integer | |
| format: int64 | |
| description: >- | |
| Seconds of inactivity before the compute endpoint is suspended. | |
| Use -1 for always on, or a non-negative value. | |
| databaseComputeSettings: | |
| databaseComputeSettingsRequest: | |
| type: object | |
| description: >- | |
| Request body for setting compute settings. | |
| All fields are optional; only provided fields are updated. | |
| properties: | |
| min_cu: | |
| type: number | |
| format: double | |
| x-nullable: true | |
| minimum: 0.25 | |
| maximum: 16.0 | |
| description: >- | |
| Minimum compute units (0.25 to 16.0). | |
| Must be less than or equal to max_cu. | |
| max_cu: | |
| type: number | |
| format: double | |
| x-nullable: true | |
| minimum: 0.25 | |
| maximum: 16.0 | |
| description: >- | |
| Maximum compute units (0.25 to 16.0). | |
| Must be greater than or equal to min_cu. | |
| max_cu - min_cu must not exceed 8.0. | |
| sleep_timeout_seconds: | |
| type: integer | |
| format: int64 | |
| x-nullable: true | |
| minimum: -1 | |
| description: >- | |
| Seconds of inactivity before the compute endpoint is suspended. | |
| Use -1 for always on, or a non-negative value. | |
| databaseComputeSettings: |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@swagger.yml` around lines 3902 - 3927, The schema for
databaseComputeSettingsRequest currently uses non-nullable numeric types so the
generated Go client cannot distinguish "unset" from zero and the generated
Validate() is a no-op; update the OpenAPI schema to mark min_cu, max_cu, and
sleep_timeout_seconds as nullable (e.g., add nullable: true) and add proper
numeric constraints (min_cu and max_cu: minimum 0.25, maximum 16.0; also
document/express that max_cu - min_cu must be <= 8.0; sleep_timeout_seconds:
allow -1 or any non-negative integer) so the generator produces pointer types,
and then implement validation logic in the model's Validate() method
(DatabaseComputeSettingsRequest.Validate) to: treat nil pointers as "not
provided", enforce min/max bounds on provided values, ensure max_cu >= min_cu
when both provided, and ensure (max_cu - min_cu) <= 8.0; keep error messages
clear and reference the field names min_cu, max_cu, sleep_timeout_seconds.
2 participants
implementation has drifted a bit, just putting it back on track