postgres create-branch: add --ttl and --no-expiry flags - #6313
postgres create-branch: add --ttl and --no-expiry flags#6313scott-mead-db wants to merge 2 commits into
Conversation
Setting a branch's expiration on `databricks postgres create-branch`
previously required a hand-written `--json` spec. Add two convenience
flags:
--ttl <duration> sets spec.ttl; accepts the REST API form (604800s),
a Go duration (168h), or day/week units (7d, 3w)
--no-expiry sets spec.no_expiry
One of --ttl, --no-expiry, or a spec expiration in --json is required;
they are mutually exclusive. The flags are wired through the existing
createBranchOverrides hook in a PreRunE that shapes req.Branch.Spec
before the generated RunE merges --json and calls the API, so the
generated command and the SDK are untouched.
Co-authored-by: Isaac
Waiting for approvalBased on git history, these people are best suited to review:
Eligible reviewers: Suggestions based on git history. See OWNERS for ownership rules. |
|
Asking for a review @andrewnester |
|
An authorized user can trigger integration tests manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
Integration test reportCommit: c16a3d1
8 interesting tests: 4 SKIP, 3 RECOVERED, 1 KNOWN
Top 6 slowest tests (at least 2 minutes):
|
|
|
||
| title "Conflicting flags fail before any API call (no request recorded)" | ||
| musterr $CLI postgres create-branch projects/acc-proj branch-conflict --ttl 1h --no-expiry &> LOG.conflict | ||
|
|
There was a problem hiding this comment.
Could you add a tst when both passed like --no-expiry --json '{\"spec\":{\"no_expiry\":false}}'
|
|
||
| title "An expiration is required: no flag fails before any API call (no request recorded)" | ||
| musterr $CLI postgres create-branch projects/acc-proj branch-none &> LOG.required | ||
|
|
There was a problem hiding this comment.
coudl you add a test for `create-branch projects/p b --json 'not json'
|
|
||
| title "the --no-wait path still works alongside --no-expiry" | ||
| $CLI postgres create-branch projects/acc-proj branch-nowait --no-expiry --no-wait > LOG.nowait 2>&1 | ||
|
|
There was a problem hiding this comment.
What happens if you pass something like --json '{\"spec\":{\"ttl\":\"\"}}'?
| if len(raw) == 0 { | ||
| return false | ||
| } | ||
| var probe struct { |
There was a problem hiding this comment.
Why a separate type instead of SDK one? This decodes differently from convert.Normalize, and if a future SDK bump renames/retypes a BranchSpec field, neither the compiler nor tests catch the drift which lead to error / unexpected behaviour
3 participants
Creating a Lakebase branch requires an expiration (no-expiry is valid, but required). Today, this is not documented and only possible via a --json spec which is unintuitive and creates friction for users.
Previous syntax:
New syntax:
This PR adds two switches to the databricks postgres create-branch command:
--ttl sets spec.ttl; accepts the REST API form (604800s),
a Go duration (168h), or day/week units (7d, 3w)
--no-expiry sets spec.no_expiry
One of --ttl, --no-expiry, or a spec expiration in --json is required; they are mutually exclusive. The flags are wired through the existing createBranchOverrides hook in a PreRunE that shapes req.Branch.Spec before the generated RunE merges --json and calls the API, so the generated command and the SDK are untouched.
Co-authored-by: Isaac