Conversation
DEFAULT_MANAGEMENT_VERSION is v2 and the test suite already deploys
clusters, but CI still drove v1 workspace groups and tore down through a
hardcoded /v1/workspaces URL, emitting a DeprecationWarning on every run.
resources/create_test_cluster.py now makes one create_cluster() call.
wait_on_active covers ACTIVE, the endpoint and the firewall, replacing
both polling loops and the bare sleep, and closing a gap: the script
never waited for the firewall, which the API applies outside the state
machine. The shared "Python Client Testing" group -- never deleted by CI
-- is gone with the flat cluster model.
Fixes a live leak: --expires was parsed and never passed to the API, so
CI clusters had no expiry and a failed shutdown job leaked a billable
deployment indefinitely. It now reaches expires_at=.
POST /v2/clusters generates the admin password and ignores what is sent;
PATCH ignores it too (docs/management-api-audit.md item 9), so
create-then-PATCH is not available and secrets.CLUSTER_PASSWORD can no
longer be the password of the cluster CI just made. The generated value
is read off the create response and propagated as a cluster-password job
output. Job outputs are not secrets, so it is ::add-mask::-ed where it is
emitted and again in every consuming job -- the mask does not cross job
boundaries, and omitting the re-mask would leak a live credential.
The generated password is drawn from the full printable set (one observed
value: {:D}TK*[F3Ll}Ups2pNv), so a percent-encoded cluster-password-url
output is emitted alongside it for the SINGLESTOREDB_URL and
CIBW_ENVIRONMENT call sites; the raw form reaches drop_db.py through the
environment rather than being interpolated into a shell word. Verified
that the encoded form round-trips through the SDK's own URL parser.
Cluster names are cleaned to [a-z0-9]([a-z0-9-]*[a-z0-9])? and truncated
to 32 characters -- CI passes a workflow name that can overrun the limit.
Regions are matched to a Region object, since v2 regions have no ID, and
the pattern is tried against both the display and provider region names.
A project is required by the API and does not auto-resolve in a
multi-project org, so --project was added, defaulting to the
STANDARD-edition project and pinnable with the CLUSTER_PROJECT variable.
drop_test_cluster.py now takes a cluster ID, matching what the create
script emits; its old contract said workspace-id but slugified the
argument into a name. The workflow teardown stays curl, so the shutdown
job needs no install, with the URL moved to DELETE /v2/clusters/{id}.
Also makes the deprecated v1 suite a nightly gate rather than a per-PR
cost: code-check.yml and coverage.yml deselect management_v1, and
coverage.yml gains a job that runs it. Verified the two selections
partition the suite exactly, 905 + 65 of 970.
Two docs gaps closed alongside: Cluster.update()'s admin_password lacked
the warning create_cluster() carries, an asymmetry that invites the
create-then-PATCH dead end; and build_docs.py rewrote workspace.Stage
but not cluster.Stage, which the v2 shim now re-exports.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The v2 clusters API returns expiresAt in Go's time.Time.String() format -- '2026-09-17 14:42:41.445984 +0000 UTC' -- while every other timestamp is RFC 3339. to_datetime split the string on '.' to pad the fractional seconds, produced '445984 +0000 UTC' as the fraction, and datetime_fromisoformat then returned None. Cluster.expires_at read as None on a cluster that had an expiry set, and SHOW CLUSTERS printed a blank expiry column. Normalize both shapes before parsing: pad or truncate the fraction to six digits, keep a numeric offset, and drop the trailing zone abbreviation and Go's monotonic-clock reading. An offset-aware result is converted to UTC and returned naive, matching what the RFC 3339 path already produced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A v2 cluster has exactly one user, admin, and no route creates another, so secrets.CLUSTER_USER could only ever hold that one value. Drop the secret and name admin directly. Name the deployment project in the workflow too, rather than reading it from vars.CLUSTER_PROJECT: the target is then visible next to the create call and an unset repository variable cannot quietly change where CI deploys. create_cluster resolves a project name against GET /v2/projects and raises with the org's project list if it matches none, so a renamed project fails loudly at setup. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The workflow files had no linter, so nothing checked expression syntax,
needs.*.outputs.* references or the shell inside run: blocks. actionlint
covers all three and ships as a pip wrapper, so the hook needs no Go
toolchain or Docker.
It reported 17 pre-existing problems, all fixed here so the hook lands
green:
- actions/checkout@v3, actions/setup-python@v4 and
docker/setup-qemu-action@v2 run on node runtimes GitHub is retiring.
Bumped to v4, v5 and v3, which is what the newer workflows already
pin.
- publish.yml named a step after ${{ matrix.python-version }} in a job
whose matrix defines only os, so the name rendered with nothing after
it. That job pins 3.10 as the host interpreter for cibuildwheel, so
the reference was never going to resolve; dropped it.
- code-check.yml left $GITHUB_OUTPUT unquoted on six redirects. Quoted.
The one remaining sed is prefixing every line, which parameter
expansion cannot do, so SC2001 is suppressed in place with a reason.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous commit bumped checkout to v4 and setup-python to v5 to satisfy actionlint, but both of those majors still declare node20, so the runners kept reporting them as forced onto node24. actionlint 1.7.7 does not know about that deprecation, so it had nothing to say. Pin the lowest major of each action that declares node24, taken from action.yml at the tag rather than from the release notes: checkout v5, setup-python v6, upload-artifact v6, setup-qemu-action v4, and download-artifact v7 -- v5 and v6 of download-artifact are still node20, so it is the one that has to skip further ahead. cibuildwheel and gh-action-pypi-publish are composite actions and never had a node runtime to move. Choosing the lowest node24 major rather than the newest keeps the behavioural change to a minimum; there is no other reason to jump to checkout v7 today. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
kesmit13
requested review from
mgiannakopoulos,
pmishchenko-ua and
volodymyr-memsql
as code owners
September 17, 2026 14:21
Follows the node24 move rather than stopping at the lowest major that cleared it: checkout v7, setup-python v7, upload-artifact v7, download-artifact v8, setup-qemu-action v4. One deprecation cycle instead of two. Checked the breaking changes in the majors this skips over: - setup-python dropped its default Python version, so a step that names neither python-version nor python-version-file now fails. All nine call sites name python-version, so none are affected. - download-artifact v5 changed the path layout for single downloads by ID. publish.yml downloads by name, so it is untouched. - download-artifact v8 stopped unzipping unconditionally, checking Content-Type first, and now errors on a hash mismatch. The artifacts here are ordinary zipped directory uploads, so both apply harmlessly. cibuildwheel and gh-action-pypi-publish stay where they are: both are composite actions, so neither was part of the node problem, and moving cibuildwheel two majors is a build-behaviour change that does not belong in this PR. The artifact pins are the ones with no coverage here -- publish.yml runs only on a tag or a release, so they are first exercised by the next release build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Go renders the offset without a separator -- the expiresAt values come back as '+0000' -- and datetime.fromisoformat only accepts that spelling on Python 3.11 and later. On 3.9 and 3.10 it raised, the converter returned the string unchanged, and to_datetime turned that into None: exactly the silent unset expiration the Go-format handling was added to fix, just on the interpreters the previous commit did not cover. Normalize the offset to +00:00. Verified that every shape the normalizer emits parses on 3.8, 3.10 and 3.11. The new test asserts on the normalized string rather than on a parsed datetime. The six existing tests were correct and still passed on 3.11, which is how this reached CI; a string comparison fails the same way on every version. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
POST /v2/clusters generates its own admin password and ignores any that is sent, so create_test_cluster.py was reading the generated one back off the create response and reporting it as a job output. That cannot work: the value has to be masked, and the runner drops any output whose value matches a mask -- "Skip output 'cluster-password' since it may contain secret" -- so the test jobs received an empty password and failed with "1045: Access denied for user 'admin'@... (using password: NO)". Take a --password instead and hand it to the new cluster over SQL once it is active, so every job reads the credential from secrets.CLUSTER_PASSWORD and nothing crosses a job boundary. ALTER USER is the statement that works; SET PASSWORD wants a 41-digit hash and rejects a literal. The percent-encoded variant and the per-job ::add-mask:: steps both go away with the output. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
code-check.yml checks singlestoredb/management and singlestoredb/fusion for changes and picks between a run that includes the v2 management tests and one that excludes them. It has always picked the second. The checkout was shallow, fetch-depth: 2, which creates no origin/main, so every diff against it died -- "fatal: bad revision 'origin/main'" appears twice in each run log -- and the `|| true` turned that into an empty file list, which reads as "nothing changed". The step that runs -m 'not management_v1' was unreachable and the -m 'not management' one always won, so the 37 live v2 management tests never ran on a pull request; only the nightly coverage.yml covered them. Fetch the full history so origin/main exists, which also repairs the branch-push path that probed origin/main and origin/master and fell through to HEAD~1. Drop the `|| true` as well: a git failure means the comparison did not happen, and swallowing it silently downgrades the run rather than reporting the breakage. Expect this job to get slower on any PR touching those directories -- the tests it now selects deploy real clusters. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A job's createdAt came back as '2026-09-18T12:39:20.43888Z'. The zone group in _GO_DATETIME_RE demanded whitespace ahead of it, so a bare Z never matched and the value fell through to the escape hatch, which strips the Z and skips the fractional-second padding. Only Python 3.11 and later read a fraction that is neither 3 nor 6 digits, so on 3.10 the converter handed back the string and to_datetime_strict raised ValueError, taking down TestJobsFusion.test_run_wait_drop_job in CI. Recognize Z as an offset so RFC 3339 goes down the same path as the Go shape and gets its fraction padded, and spell the offset out as +00:00 for the same reason the numeric ones grew a colon: nothing before 3.11 parses the short form. _as_naive_utc shifts it back off, so parsed results are unchanged. Verified on a real 3.10 that every shape the normalizer emits parses, and that the old output for this value does not. The tests assert on the normalized string, not on a parsed datetime, so they fail on 3.11 as well -- the same blind spot that let the offset bug reach CI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The matrix stopped at 3.13 while 3.14 has been final since October 2025, so the newest interpreter the package claims to support -- requires-python is >=3.9 with no ceiling -- went untested. Crossed with the driver axis this adds two jobs, mysql and https. 3.15 is left out on purpose: it is at rc2 today with GA planned for 2026-10-01, and setup-python will not resolve a bare "3.15" until then. The condition for adding it is recorded above the matrix. Not touched: the include: block still pins macOS and Windows to 3.11, so 3.14 is covered on Linux only, and publish.yml builds one abi3 wheel from cp39, which needs no change for a new minor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Raw password URLs can fail for valid secrets, and setup failures can leave created clusters without a cleanup ID.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Retargets CI provisioning from v1 workspaces to v2 clusters, while improving timestamp parsing and workflow validation.
Changes:
- Adds v2 cluster creation, expiration, project selection, password setup, and teardown.
- Normalizes RFC 3339 and Go-formatted timestamps with regression tests.
- Updates workflows, action versions, test markers, documentation transforms, and actionlint integration.
File summaries
| File | Description |
|---|---|
resources/create_test_cluster.py |
Creates and initializes v2 test clusters. |
resources/drop_test_cluster.py |
Terminates clusters by ID. |
singlestoredb/management/utils.py |
Normalizes management timestamps. |
singlestoredb/tests/test_management_utils.py |
Tests timestamp parsing behavior. |
.github/workflows/smoke-test.yml |
Runs v2 cluster smoke tests. |
.github/workflows/publish.yml |
Uses v2 clusters for wheel tests. |
.github/workflows/coverage.yml |
Separates v1 nightly coverage. |
.github/workflows/code-check.yml |
Improves change detection and test selection. |
.github/workflows/pre-commit.yml |
Updates workflow actions. |
.github/workflows/fusion-docs.yml |
Updates documentation workflow actions. |
.pre-commit-config.yaml |
Adds actionlint. |
resources/build_docs.py |
Normalizes cluster stage links. |
singlestoredb/management/v2/cluster.py |
Documents ignored admin-password updates. |
Review details
Suppressed comments (1)
.github/workflows/smoke-test.yml:154
- The HTTPS matrix uses the same raw password interpolation as the MySQL matrix, so a secret containing
+,@,/,?,#, or%is not the password the client receives after URL parsing. Encode this userinfo component or supply the credential outside the URL rather than relying on an unenforced alphanumeric secret convention.
SINGLESTOREDB_URL: "${{ matrix.driver }}://admin:${{ secrets.CLUSTER_PASSWORD }}@${{ needs.setup-database.outputs.cluster-host }}:443/${{ needs.setup-database.outputs.cluster-database }}?pure_python=${{ matrix.pure-python }}&buffered=${{ matrix.buffered }}"
- Files reviewed: 13/13 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Two findings from review. resources/create_test_cluster.py reported the cluster ID after the password reset and the SQL load, both of which can fail against a cluster that is already running and already billing. A CI teardown job would then have an empty cluster-id output and send its DELETE to /v2/clusters/, leaking the cluster. Everything the reporting block prints is known as soon as create_cluster() returns, so it now runs there. The two workflow shutdown steps also refuse to issue a DELETE with an empty ID, and --fail-with-body makes a refused one fail the step. to_datetime read Go's zero time as year 1 whenever it arrived in the Go shape -- '0001-01-01 00:00:00 +0000 UTC' -- because only the RFC 3339 spelling was compared against. That reports an expiry on a resource that does not expire. Both helpers now test the parsed value for January 1 of year 1, which covers every spelling including the offset, zone name and monotonic reading, and the check runs before the UTC shift, which can fall below MINYEAR on a year-1 value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…xture Three changes to what the management suites deploy. expires_at on every creation that accepts one. The sweep, the ledger and tearDownClass are all in-process, so none of them survives the runner being killed -- and GitHub force-terminates a cancelled job's remaining steps after five minutes, which is less than the ~460s an S-00 takes to become deletable. An expiry is a property of the deployment, so the control plane honours it either way. resources/create_test_cluster.py has passed one nightly since it was written; this is the same argument applied to the fixtures. It reaches v2 create_cluster and v1 create_workspace_group -- a v1 workspace has no expiry of its own and goes with its group, and the starter variants take no such argument. TestClusterFusion borrows from the shared pool instead of deploying three clusters of its own. It is four SHOW statements and mutates nothing, which is what the pool asks of a consumer. Net two fewer clusters per run: the pool grows 2 -> 3, and three per-class deployments go away. It joins the Stage group rather than the Jobs one because Stage already asks for two, so the growth costs one cluster rather than two. Its assertions now scope to shared_cluster_pattern() and count against shared_cluster_names() rather than a literal 3, so a later class asking for a bigger pool cannot break them. test_create_cluster_named_project terminated through a bare terminate(force=True) whose comment said force makes a PENDING cluster deletable. It does not -- the API refuses it with a 400 or 409 either way, which is the whole reason utils.terminate retries -- and the except swallowed the refusal, so the usual outcome was a cluster left to the sweep. Now calls utils.terminate, as its sibling test already did. The neighbouring claim that Fusion-created clusters are invisible to tracking was also wrong: the handler goes through ClusterManager.create_cluster, which is the method _CREATORS wraps. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The v1 management API refuses a create_workspace_group with "could not acquire lock" when another creation in the organization holds it, and nothing retried that: Manager.RETRY_STATUSES covers 429 and 5xx only, so the error reached setUpClass, where one failure fails every test in the class. The last management_v1 run went that way in bulk. The lock clears on its own in seconds, so utils.create_retrying() waits and asks again -- six attempts spaced 20/40/60/60/60s with up to 5s of jitter, matched on the error message rather than on a status. Spacing is sized for lock contention, not provisioning: a group POST returns in seconds, so what is being waited out is the other holder finishing, not a deployment coming up. The jitter is there because two xdist workers -- or the v1 nightly beside a concurrent v2 job -- back off by the same amounts from the same moment, and would otherwise retry in step forever. Four minutes at worst, small enough that a genuinely stuck organization fails the class instead of idling out the job's timeout. Applied to the four v1 setUpClass creations: the three in test_management_v1.py and TestWorkspaceFusion, which deploys three groups in a row and is the likeliest source of the contention. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The same "could not acquire lock" 500 comes back from POST /clusters -- the v2 shared cluster pool lost TestClusterFusion to it an hour after the v1 job's workspace groups went the same way. Note the server says "error creating workspace" for a /clusters POST, so the match cannot key on the noun; both observed wordings are now asserted verbatim. So create_retrying() now wraps every live creation, not just the v1 workspace groups: the pool build in utils.shared_clusters (where a raise fails every class that borrows from it), _ClusterFusionMixin, v2 TestCluster and TestStarterCluster, and v1's create_workspace and create_starter_workspace. Retrying at the call site rather than by adding POST to RETRY_METHODS is the point: POST is excluded there deliberately, since a retried POST can create twice, and the transport sees only a 500. The lock message is the evidence that this particular POST created nothing, which is what makes reusing the name safe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The two previous commits retried "could not acquire lock" from the test suite, wrapping each creation call in utils.create_retrying(). That only covers the call sites someone remembered to wrap, and nothing outside the tests -- a Fusion SQL CREATE CLUSTER, or any SDK user -- got it at all. So the wait now lives on the two methods that contend for the lock: manager.retry_on_lock, worn by v1 WorkspaceManager.create_workspace_group and v2 ClusterManager.create_cluster. Fusion SQL's CREATE WORKSPACE GROUP and CREATE CLUSTER go through those same two methods, so they are covered without a second implementation, and so is every setUpClass that used to need the wrapper. Deliberately those two and nothing else, rather than a hook in Manager._doit that would have covered every route and verb: only these two take the lock, and a retry on everything is a blocking wait bolted onto calls that should just fail. Which methods wear it is asserted in test_management_utils.py rather than left to a reader to notice. Budget and matching are unchanged from create_retrying: keyed on the error message, 20/40/60/60/60s with up to 5s of jitter, now tunable through SINGLESTOREDB_MANAGEMENT_LOCK_RETRIES and SINGLESTOREDB_MANAGEMENT_LOCK_RETRY_INTERVAL, with 0 retries restoring the old fail-fast behaviour. Replaying a POST is still safe for the same reason it was there: RETRY_METHODS excludes POST because the transport sees only a 500 and cannot know whether the server acted, whereas the lock message says the creation never started. The waits go through timing.sleep, so a traced run reports them as "create_cluster organization lock" instead of losing them to unaccounted time. create_retrying() and its ten call sites are gone, and its unit tests now drive the decorator. The two TestSharedClusterPool lock tests are dropped rather than moved: their stand-in manager raises ManagementError from a fake create_cluster and never reaches the decorated method, so there was nothing left for them to assert. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The prose blocks this branch added had grown past the point where anyone
would read them. Condensed them across the management API, the test
helpers and the workflows, keeping the load-bearing facts -- the run IDs,
the 5-minute cancellation timeout, the ~460s ACTIVE floor, the audit item
references -- and dropping the restatements.
Three substantive fixes along the way:
* the shared-pool comment in test_management_utils.py named a class
that does not exist (TestLockRetry, not TestManagerLockRetry) and put
the lock retry in Manager._doit, where it is a @retry_on_lock
decorator on create_cluster. Both wrong since c24243b.
* _run_ledger_sweep returned 0 from its dry run even with unresolved
records, while the no-leftovers path returned 1 for that same
condition. An unresolved record means a deployment that may still be
live, so a dry run should say so in its exit status too.
* create_test_cluster.py's --password help pointed at a comment with
"(see below)", which means nothing in --help output.
Also dropped a dead `live = []` in test_fusion.py, whose `finally`
always assigns it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TestSecrets used a fixed secret name and opened by deleting any leftover of it. A secret is org-scoped, so that leftover is a concurrent run's live secret as often as a stranded one: two runs at once each deleted what the other had just created. Name it per-run instead, which makes the leftover-clearing delete unnecessary and the in-test delete the cleanup. Nothing else sweeps a secret as it is made, so a run killed between the POST and the DELETE strands one for good. Add a --secrets mode to cleanup_deployments.py for those, age-guarded so it cannot reap the run calling it, and run it from the always() cleanup step in both workflows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The suite built one as secrets.token_urlsafe(20) + '-x&$'. The token half can contain 'abc' or '321', which the API rejects for more than two consecutive sequential characters -- a low enough rate to read as a flake. Generate the whole password instead, redrawing any character that would close a run of three and requiring the mix of cases, digits and a special character the policy wants. Which characters count as special was probed against POST /v1/workspaceGroups, which checks the password before it looks the region up, so a nonexistent region ID makes an accepted password surface as the region's 404 and creates nothing: '-', '_', '$', '!', '@', '#', '%' and '*' all satisfy the rule and '&' does not. The old suffix passed on its '-' and '$'. Draw the specials from '-_$', the three that are also URL-unreserved or a sub-delimiter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This branch was successfully deployed
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
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.
Retargets the CI provisioning scripts and workflows from the v1
workspace-group flow at v2 clusters, which is what
management.versionalready defaults to.
Provisioning
resources/create_test_cluster.pyis now a singlemanage_clusters()/create_cluster()call.wait_on_active=Truecovers ACTIVE, then theendpoint, then the firewall, which replaces both polling loops and a bare
time.sleep(10).--expireswas parsed and then never used, so CIclusters had no expiry at all; it is wired to
expires_atnow.resources/drop_test_cluster.pytakes a cluster ID and terminates it.The admin password needs care.
POST /v2/clustersaccepts anadminPasswordand ignores it, generating its own, andPATCHdoes thesame, so the password can only be read off the create response and is
Noneafter any refresh. That value cannot reach a consuming job: it hasto be masked, and the runner drops any output whose value matches a mask
-- "Skip output 'cluster-password' since it may contain secret" -- so
masking it and passing it between jobs are mutually exclusive.
So the generated password is traded for
secrets.CLUSTER_PASSWORDoverSQL, once, while the script still holds it:
ALTER USER 'admin'@'%' IDENTIFIED BY ...(SET PASSWORDwants a pre-hashed value and rejects aliteral with
1372: Password hash should be a 41-digit hexadecimal number). No password is reported at all after that, and every consumingjob reads the secret it already had. Keeping the credential a secret
rather than a job output is also why
SINGLESTOREDB_URLinterpolates itraw: a percent-encoded copy is a different string from the mask, so
emitting one would put a live credential past the runner's masking. The
secret is maintainer-set, so keeping it alphanumeric is a cheaper
constraint than that; both consuming sites say so.
The ID, host, port and database name are reported as soon as
create_cluster()returns, before that password reset and before the SQLload. Both can fail against a cluster that is already running and already
billing, and a teardown job with an empty
cluster-idwould send itsDELETE to
/v2/clusters/and leak it. The two shutdown steps refuse anempty ID outright now.
secrets.CLUSTER_USERandvars.CLUSTER_PROJECTare gone. A cluster hasexactly one user,
admin, and no route creates another, so the secretcould only ever hold that value; the project is named in the workflow so
the deployment target is visible next to the create call. Once this
merges,
CLUSTER_USERcan be deleted from repository settings.A datetime bug this uncovered
expires_atread back asNoneon a cluster that demonstrably had anexpiry set.
expiresAtis the one field the API returns in Go'stime.Time.String()format --2026-09-17 14:42:41.445984 +0000 UTC--while every other timestamp is RFC 3339.
to_datetimesplit on.topad the fraction, produced
445984 +0000 UTC, and parsing returnedNone. Pre-existing, and it also left the expiry column blank inSHOW CLUSTERS. Fixed with tests for both shapes.Accepting that shape then exposed its zero value: an unset Go
time.Timerenders as0001-01-01 00:00:00 +0000 UTC, and only theRFC 3339 spelling of that sentinel was recognized, so a resource that
does not expire reported an expiry in year 1. Both
to_datetimeandto_datetime_strictnow test the parsed value for January 1 of year 1,which covers every spelling, and do it before the UTC shift, which can
fall below
MINYEARon a year-1 value.Workflow linting
There was no linter over
.github/workflows/, so nothing checkedexpression syntax,
needs.*.outputs.*references or the shell inrun:blocks.
actionlintis now a pre-commit hook -- a pip wrapper, so no Gotoolchain or Docker -- and the 17 pre-existing findings it reported are
fixed: stale action majors, a step named after
${{ matrix.python-version }}in a job whose matrix only definesos, andsix unquoted
$GITHUB_OUTPUTredirects.Every action pin is also off the Node.js 20 runtime now. The first pass
bumped to
checkout@v4/setup-python@v5, which satisfied actionlintbut still declared node20; the pins are the lowest major of each action
that declares node24, read from
action.ymlat the tag. Verified by therun annotations disappearing between 6c22959 and 8828326.
Verified
test.sqlloaded,expires_atexactly1h after
created_attest_basicsagainst that cluster: 30 passed, 2 skippeddrop_test_cluster.pyto TERMINATED in 2.8s,and the workflow's
curl DELETE /v2/clusters/{id}?force=truereturning200. No clusters leaked
Not verified
The reordered reporting block is not covered by a test, and the live
create above predates it. The
publish.ymlartifact-action bumps onlyexecute on a tag or release, so they land untested here.
🤖 Generated with Claude Code
Note
Medium Risk
Changes real-cluster CI provisioning, teardown, and management API create/delete behavior; failures or cancelled jobs can still strand billable resources despite new ledger cleanup.
Overview
Moves CI database setup from v1 workspace groups to v2 clusters (
create_test_cluster.py/drop_test_cluster.py,DELETE /v2/clustersin publish/smoke workflows), resets the API-generated admin password toCLUSTER_PASSWORDover SQL, and standardizes connections on useradminwith an explicit--project.Adds deployment leak prevention for management tests: a per-job JSONL ledger (
SINGLESTOREDB_TEST_DEPLOYMENT_LOG),cleanup_deployments --ledger/--secretsinif: always()CI steps, longer terminate retries, and xdist hooks so stranded clusters surface on the controller. code-check fixes the change detector (fetch-depth: 0, no silentgit diffswallowing) and excludesmanagement_v1from PR pytest; coverage runs v1 management in a separate nightly job.Management client: organization lock retry on
create_cluster/create_workspace_group,force=true|falsequery strings for deletes, andto_datetimeparsing for Gotime.Timestrings and zero-time sentinels. Test tuning includes pytest-xdist-n 2, fusion tests using the shared cluster pool, actionlint in pre-commit, and bumped GitHub Actions pins.Reviewed by Cursor Bugbot for commit b13dcd0. Bugbot is set up for automated code reviews on this repo. Configure here.