Add versioned management API wrappers - #126
Open
kesmit13 wants to merge 68 commits into
Open
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Adds the accessors the cluster grammar needs, alongside the existing v1 workspace ones: - get_cluster_manager(), pinned to v2 for the mirror image of the reason get_workspace_manager() is pinned to v1 -- the CLUSTER commands are the v2 vocabulary, and at v1 there is no cluster resource at all. - get_cluster() / get_starter_cluster(), flat counterparts of get_workspace() with no containing group to resolve first. - get_project(), which returns None when no IN PROJECT clause was given so create_cluster falls through to _resolve_project_id(). get_deployment() is repointed in place to v2. stage.py is its only consumer -- confirmed -- so the workspace handlers are unaffected. The group/in_group keys stay wired so IN GROUP keeps parsing as a synonym. SINGLESTOREDB_WORKSPACE_GROUP now raises naming SINGLESTOREDB_CLUSTER rather than resolving: v2 has no addressable group resource and Cluster.group_id is not a lookup key, so guessing could target the wrong deployment. Adds _is_missing(), because the v2 routes surface a malformed ID as 400 'uuid: incorrect UUID length' where v1 gave 404 -- probed live. Both mean the caller named something nonexistent, so both become KeyError instead of leaking a raw 400 for what is usually a typo; other 400s still propagate. Also rewords the two stale SINGLESTOREDB_CLUSTER raises that claimed clusters "are not currently supported", and documents why the inference API manager stays on v1 while files and jobs move. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Eleven handlers in a new fusion/handlers/cluster.py, pinned to management API v2 through get_cluster_manager(): SHOW CLUSTERS, SHOW CLUSTER REGIONS, SHOW PROJECTS, CREATE/DROP/SUSPEND/RESUME/USE CLUSTER, and the three STARTER CLUSTER commands, which have no v1 equivalent at all. Kept in its own module rather than added to workspace.py, which is pinned to v1: a file per version means retiring v1 is a deletion, and it removes the chance of reaching for the wrong manager mid-module. The grammar follows the v2 resource model rather than transliterating the workspace commands: * No IN REGION ID anywhere. v2 assigns no region IDs, so SHOW CLUSTER REGIONS reports no ID column and a region is named instead. Names match against both the display name and the provider slug, since GET /v2/regions reports both and a cluster's own region field is the slug. * No IN GROUP on the single-cluster commands. A cluster is flat. * No FORCE on DROP CLUSTER -- at v1 it meant "drop the group despite its workspaces", and there are no children to override. * No WITH PASSWORD yet; that waits on the password probe. * SHOW CLUSTER REGIONS, never a bare SHOW CLUSTER: the registry matches longest key first, so a two-word key would hijack SHOW CLUSTER STATUS. * NON_PRODUCTION in the grammar, mapped to the API's NON-PRODUCTION, because grammar keywords cannot contain a hyphen. CREATE CLUSTER returns a row where CREATE WORKSPACE GROUP returned none. The API generates the admin password and reports it exactly once, at creation, so a caller who cannot see it has no reachable admin user. Two bugs found while verifying the grammar parses: * visit_number() read the first flattened child, but the fraction group in the number regex is optional, so a bare integer flattened to '' first and <number> could only parse a value with a decimal point. WITH SCALE FACTOR 1 raised ValueError. Now reads the matched text. * The AUTO SUSPEND AFTER clause parses to one flat dict, not a list of one dict per sub-rule. CreateWorkspaceHandler indexes it as a list, so CREATE WORKSPACE ... AUTO SUSPEND raises KeyError: 0 today. Left alone here -- it is a v1 handler and outside this change -- but not copied. Verified live, read-only: all five SHOW forms answer, region resolution matches both spellings and passes an unknown literal through, and project lookup resolves by name and ID. All fourteen grammar forms parse.
get_deployment() already resolves against v2, so Stage was reachable on a cluster but only by spelling it IN GROUP -- naming a cluster with the vocabulary of the resource v2 replaced. Each handler's `in` alternation now offers in_cluster as well, ordered ahead of the bare in_deployment. IN GROUP stays as a parsing synonym rather than being removed: it is the spelling every existing script uses, and at v2 both resolve to the same cluster. Verified that all three spellings still parse on all six handlers.
get_files_manager() was calling manage_files() bare, which follows the management.version option -- so which API the FILES commands addressed depended on a setting that has nothing to say about files. Now pinned to v2 explicitly, like the neighbouring managers. management/files.py is version-neutral: the personal, shared and models spaces are the same resource at both versions and only the URL differs. So this changes the URL, not the implementation. Verified live: all three spaces list against https://api.singlestore.com/v2/. Its own commit so that a files regression stays distinguishable from the jobs move that follows.
All eight call sites went through get_workspace_manager(), which is pinned to v1 by design because the WORKSPACE commands are v1 vocabulary. The JOB commands are not: a job runs against a deployment, and at v2 a deployment is a cluster. Routing them through the v1 manager meant every scheduled job carried a v1 targetType. The move is real rather than cosmetic. JobsManager encodes the version difference as class attributes, and the two managers report different values: v1: Workspace / VirtualWorkspace / Cluster v2: Cluster / VirtualCluster / None The legacy target type is None at v2 because there is no third kind of target left to name. Verified live: the runtimes route answers through the v2 manager. Its own commit so that a jobs regression stays distinguishable from the files move that precedes it.
…ettle
Audit item 14: create_workspace_group posts adminPassword,
backupBucketKMSKeyID, dataBucketKMSKeyID and smartDR. POST /v2/clusters has
none of the last three and ignores the first, and only
highAvailabilityTwoZones survives, renamed multiAZ. So CREATE CLUSTER offers
no WITH PASSWORD, KMS-key or SMART DR clause -- a clause for any of them
would parse, be sent, and be dropped silently, which reads as though it had
taken effect.
Two questions are left open rather than answered, because the throwaway
cluster the probe needed could not be created in this environment:
* whether PATCH /v2/clusters/{id} honours adminPassword. Acceptance proves
nothing on its own -- item 9 records the same route accepting and ignoring
name -- so it needs a real connection with the patched value. If it does,
WITH PASSWORD becomes implementable as create-then-PATCH.
* re-confirmation of item 8. It was confirmed 2026-08-21, but finding 6 has
since been corrected from a live probe, so one v2 assertion in this
document has already proved wrong.
Also softened the DROP CLUSTER docstring. It asserted that FORCE has nothing
to override at v2; DELETE /v2/clusters does still take a force query
parameter, documented with a different meaning ("even if it is in use") that
was never confirmed. Stated as reasoning now, with a pointer to item 14.
TestFusion gains 10 tests that need no token, so they run in CI under -m 'not management' -- the cheap regression net for the parts that can be checked without a deployment: * the eleven commands are registered, and SHOW CLUSTER STATUS still routes to None. That last one guards the whole reason the region command is spelled SHOW CLUSTER REGIONS: the registry matches longest key first, so a bare two-word SHOW CLUSTER would swallow an engine command. * CREATE CLUSTER's rendered syntax has no region-ID alternate and no KMS, SMART DR or PASSWORD clause, while CREATE WORKSPACE GROUP still has both a region ID and KMS. Asserted against handler.syntax rather than the output of SHOW FUSION GRAMMAR, which also carries the prose remarks -- and those mention the absent clauses in order to explain the absence. * a maximal CREATE CLUSTER parses, pinning both bugs the last commit fixed: WITH SCALE FACTOR 1 must yield 1.0, and auto_suspend must be one flat dict. * all six Stage handlers accept IN CLUSTER, IN GROUP and a bare IN, with in_cluster ahead of in_deployment in the alternation. * each Fusion manager names its version rather than following the option, and job.py has no get_workspace_manager left. TestClusterFusion is the live mirror of TestWorkspaceFusion, flat rather than nested: no group fixture and no IN GROUP anywhere. Names are lowercase and hyphenated because POST /v2/clusters enforces [a-z0-9]([a-z0-9-]*[a-z0-9])? at 1-32 chars (audit item 7), so the spaced names the v1 fixture uses are rejected outright. Three clusters are created and shared across the read-only tests rather than one per test. It covers SHOW CLUSTERS in every form, SHOW PROJECTS, SHOW CLUSTER REGIONS (asserting no ID column, which doubles as the live check on region shape), create and drop by name and by ID with IF EXISTS/IF NOT EXISTS, suspend/resume, IN PROJECT named and omitted, and IN REGION ID failing to parse. Switched suites: * TestStageFusion creates two clusters instead of two workspace groups, and sets SINGLESTOREDB_CLUSTER -- SINGLESTOREDB_WORKSPACE_GROUP would now raise, because v2 has no addressable group resource and get_deployment() refuses to guess which cluster was meant. The four spellings of the deployment clause are now exercised as six, IN CLUSTER included. * TestJobsFusion creates one cluster where it used to create a group plus a workspace, and its two targetType assertions change from 'Workspace' to 'Cluster' -- the assertion that proves the manager actually moved. * TestFilesFusion loses its deployment fixture entirely rather than converting it. The personal, shared and models spaces are org-scoped and no test in the class ever referenced the workspace group setUpClass created; it was a billable resource created for nothing. TestWorkspaceFusion is untouched and verified byte-identical to the previous commit. The live suites have not been run. Creating a cluster is blocked in this environment, which is the same wall the step 4 probe hit, so TestClusterFusion and the three switched suites are unexercised -- expect iteration, especially in create_cluster's POST body, which no test has ever sent for real.
Every CREATE CLUSTER in a multi-project organization has to say which project to deploy into, and until now that meant a UUID the caller had to go look up. _project_id_for() now takes either: a UUID is used as-is, and anything else is matched against the project names in the organization. Telling the two apart by shape is safe rather than a guess. Sending a project ID that is not a UUID comes back as 400 uuid: incorrect UUID length, so a non-UUID string could never have been a valid ID and nothing is lost by reading it as a name. Keeping an explicit ID on the UUID path also means it still costs no GET /v2/projects and still works for a token that cannot list projects. This applies wherever a project can be named: the project_id argument to create_cluster() and create_starter_cluster(), and the SINGLESTOREDB_PROJECT environment variable. Fusion's IN PROJECT clause already accepted a name. The API does not promise project names are unique, so an ambiguous name raises and lists the matching IDs instead of taking the first. An unknown name raises listing the organization's projects, which is the same courtesy the more-than-one-project error already extended. Verified live: 'Standard Project' and its UUID both resolve to the same ID, and both error paths report what was found. The fake project IDs in the mocked tests become UUID-shaped. They have to be, now that shape is what distinguishes a name from an ID -- 'pr-1' would be read as a name and send the manager off to list projects mid-unit-test. That is also closer to what the live API returns.
Continues the v2 cluster work on this branch. Three threads, none large enough to stand alone: ttl_property cached on the descriptor rather than per instance, so two managers holding different tokens could be served each other's value. The cache is now keyed per object, and reset() takes the instance whose entry to drop. _project_id_for() accepts a Project object as well as a name or a UUID, and Cluster grows a project_id property, so a caller who already holds the object does not have to reach into it. Fusion's SHOW CLUSTERS reads region and project through helpers, since a v2 Cluster reports the provider slug and has no region object to ask. The audit doc records two findings re-confirmed against the 1.2.171 spec dump (2026-08-25): GET /v2/projects is documented now where the 1.1.124 snapshot omitted it, and the generated admin password is the real credential -- proven this time by connecting with it, where the earlier evidence only showed the create response echoing a different value than was posted. The sent password is discarded, not merely unreported, which makes it a server-behavior bug worth raising rather than a missing parameter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four management tests were failing with requests.ConnectionError
('Connection aborted.', RemoteDisconnected('Remote end closed connection
without response')), and workspace groups and clusters were surviving the run
that made them. Two separate causes.
Manager had no retries and no request timeout at all. RemoteDisconnected means
the socket died before a response came back, which is what a keep-alive
connection closed by the far end looks like on its next use -- and the failing
tests are precisely the ones that poll a create for twenty minutes, so they
get the most chances to hit it. The session now mounts a Retry (4 attempts,
0.5s backoff) covering GET/HEAD/OPTIONS/PUT/DELETE and 429/500/502/503/504,
and applies a (10s, 180s) default timeout so a stalled connection fails and is
retried instead of hanging.
POST is deliberately excluded: a dropped connection does not say whether the
server acted on the request, and replaying POST /clusters deploys twice.
Everything the wait_on_* loops issue is a GET, which is where these failures
land. raise_on_status is off so _check still raises ManagementError with the
response body rather than urllib3 raising a bare MaxRetryError. Transport
failures are wrapped naming the method and route, so the next one is
diagnosable instead of an anonymous ConnectionError.
The leak is not a missing terminate() call: unittest does not call
tearDownClass when setUpClass raises, so TestClusterFusion dying on its second
of three create_cluster calls stranded the first one for good. Creations are
now tracked as they happen -- the creation methods are wrapped, so a new test
cannot leak by forgetting to register -- and conftest sweeps each class's
leftovers as the run moves to the next one, then everything at session end.
That path only ever holds objects made in this process, so it cannot see or
touch a parallel run's deployments.
cleanup_deployments.py handles what earlier runs already stranded. It is
organization-wide and matches on names, and a name identifies the suite but
not the run, so a concurrent run's fixtures look exactly like stranded ones.
Age is the only thing separating them: --older-than defaults to 6h rather than
0, an unreported creation time is spared rather than swept, and a naive
timestamp is read as UTC instead of local time, which east of UTC would
overstate the age and sweep something a live run owns. It reports what it
declined to touch, so a skip cannot read as nothing being there. A per-run id
in the names would be better, but v2 caps cluster names at 32 characters and
a-fusion-cluster-<8 hex> already spends 25.
Verified: 50 unit tests over the retry policy, the sweep and the age guard; a
throwaway two-class probe confirmed the per-class sweep fires between classes;
one stranded workspace group found and terminated live. The transport change
itself is unverified against the API -- the original failure was never
reproduced, and doing so needs a token and a 40-minute run.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A traced run spent 8874s of 8915s inside the management API, 5145s of that asleep in wait_on_* loops. There is no local work to optimize, so the lever is deploying fewer clusters and overlapping the ones that remain. Test performance: * singlestoredb/management/timing.py: time every management request and every wait_on_* poll behind the new management.trace option (SINGLESTOREDB_MANAGEMENT_TRACE). manager.py records at the single choke point every HTTP call passes through. conftest.py adds per-test and per-class traces -- the class one matters most, since setUpClass is where the clusters get deployed -- and prints a per-route breakdown in the terminal summary. * utils.shared_clusters(): a lazily built, process-wide pool of v2 clusters for the classes that need nothing but a live deployment. TestStageFusion, TestJobsFusion and v2's TestStage/TestJob move onto it: five clusters and 2190s of fixture time become two clusters and ~890s. The pool is created with the owner cleared so the per-class deployment sweep does not eat it after its first consumer. * pyproject.toml: -n 3 --dist loadgroup in addopts. loadgroup is load-bearing -- xdist's default load splits a unittest class across workers and each one runs setUpClass itself, turning one shared fixture into N deployments. The xdist_group marks keep the two pool consumer groups together. 3 rather than auto because the ceiling is the API's tolerance for concurrent provisioning, not the host's CPUs. * publish.yml: cibuildwheel now needs pytest-xdist, since its run picks up this pyproject as the inifile. Also in here, because it shares the same files: * management.version and the default_version attributes flip to v2. v1 stays reachable by option or factory argument until management/v1/ goes. * v2 Cluster parses sizeConfig as well as size, for the field rename. * CREATE CLUSTER drops WITH DEPLOYMENT TYPE and ENABLE MULTI_AZ, which the v2 route does not accept; ClusterManager.create_cluster still takes both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The models surface is v1-only. START/STOP/SHOW/DROP MODEL ride the inferenceapis/ routes, which do not respond past v1, and the CUSTOM MODEL commands are the same generation of API. Rather than let the Fusion grammar advertise commands with no v2 equivalent, mark all eight handlers _enabled = False so they only register under SINGLESTOREDB_FUSION_ENABLE_HIDDEN, the same escape hatch export.py uses. Delete management/inference_api.py, the top-level shim over v1/inference_api.py. A v1-only resource should not be reachable under a version-neutral name, so Fusion and singlestoredb.ai now import from management.v1.inference_api directly, and Organization.inference_apis drops out of the published autosummary. Organization._inference_api_manager_class stays None on the shared base and .inference_apis still raises for every version past v1 -- that part was already where it needs to be. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
_is_mocked() returned True -- not tracked, not swept -- for any object whose _manager was None, which is the opposite of the policy its sibling _creator_is_mocked() documents and the wrong way round for the risk: a fake deployment swept is a round trip and a warning, whereas a real one skipped is a cluster left running and billing. Drop the branch entirely rather than inverting it. _creator_is_mocked() already has the right bias -- given None it finds no _post and no mocked _get/_post/_delete, so it answers "real" -- so falling through to it needs no new logic. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
trace_management_api appended to _management_traces only when the test had recorded a management event, but trace_management_api_class derives the fixture share by subtracting exactly that list from the class total. A test that made no management call was therefore never subtracted and its wall clock landed on setUpClass, which is the figure the trace summary exists to report. Append unconditionally so the arithmetic sees every test, and filter the event-less ones out at report time through the new _traced() helper -- the combined total and both "slowest" listings all use it, since an empty trace would otherwise inflate elapsed and unaccounted. Separately, pytest_runtest_setup read item.module directly, which is an attr-defined error under a full `mypy singlestoredb/`. pre-commit's mirrors-mypy runs with only types-requests, so pytest.Item degrades to Any there and the error was invisible. Hoisting the getattr to a local fixes it: 112 errors -> 111, the rest pre-existing third-party noise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fusion/handlers/utils.py and v2/cluster.py each read SINGLESTOREDB_WORKSPACE and SINGLESTOREDB_PROJECT out of os.environ directly, while management/utils.py already exposes get_workspace_id() and get_cluster_id() for exactly that. Route the five reads through the accessors so the env-var names live in one module. SINGLESTOREDB_PROJECT had no accessor, so add get_project_id() next to the other three rather than leave a literal read behind in a different package. That is one function mirroring get_workspace_id(), not the CLUSTER_ENV_VARS / CLUSTER_GROUP_ENV_VAR / PROJECT_ENV_VAR constants the plan docs describe -- those never existed under those names, and CLUSTER_ENV_VARS itself was deleted in 3a9ebb0 once one variable was left. Both plan docs now say so. SINGLESTOREDB_WORKSPACE_GROUP is untouched: the one site that checks it never reads its value, it deliberately refuses to resolve, and there is no accessor to route it through. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
api.rst documented only management API v1, but management.version now defaults to v2 -- so the published reference described the non-default API, and manage_clusters, ClusterManager, Cluster, StarterCluster and Project appeared nowhere. Add a cluster section mirroring the workspace section's structure, ahead of it, and retitle the workspace half "Workspaces (v1)" with a note that a bare manage_workspaces() call is now deprecated. The version-neutral sections that reached their manager through WorkspaceManager -- Region, Organization, Stage Files -- now name the ClusterManager/Cluster attribute first and the v1 one second. Same for FilesObject's docstring, which pointed only at WorkspaceGroup.stage. management.timing is deliberately left out: it is internal. Every autosummary entry was checked to resolve against the source. Not Sphinx-built: docs/src/Makefile's html target starts a Docker container and moves its output over the committed HTML in docs/. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ADR 0001 listed JobsManager._legacy_cluster_target_type, which does not exist -- the overrides are _deployment_target_type and _starter_target_type (management/v1/job.py:34-35) -- and claimed "v2/stage.py is a plain re-export" when there is no v2/stage.py at all; v2's Stage comes from management/stage.py via v2/cluster.py. Also: _wait_on_endpoint's `out` parameter is documented as a Workspace, but the function is version-neutral and takes whatever deployment has a connect method. And .flake8's per-file-ignore named management/inference_api.py, which moved to v1/inference_api.py and is already covered by the v1/*.py glob two lines down. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The plan docs were written before the code and not re-read afterwards, so they still read as open questions with the answers sitting in management-api-audit.md. wait-until-usable-plan.md: all six steps landed and none was annotated. Step 6 was the one held for a decision -- it was taken as recommended but shipped differently, because the Part 7 flip removed the v1 default the proposed snippet was stepping around, leaving the shared _resolve_version(version, default=DEFAULT_CLUSTER_VERSION). fusion-v2-cluster-plan.md: step 4's probe ran on 2026-08-25. Neither POST nor PATCH honours adminPassword, so WITH PASSWORD is not implementable and is not offered; DROP CLUSTER FORCE was likewise dropped, and the handler docstring says why. The "expect 45 -> ~56 commands" estimate was exact -- verified 45 on main, 56 when the 11 cluster commands landed -- and the registry now holds 48 because 0b0765f later hid the eight inference and MODEL commands. shared-deployment-pool-plan.md: "Two things parallelism does not fix, and one it breaks" introduces four bullets, of which one is not-fixed and three are breaks. untwist-v1-v2-management-plan.md: records the export.py deferral with its real precondition (the EXPORT Fusion grammar, not cluster support in general), confirms whatsnew stays /bump-version-generated and spells out the five breaks the release commit messages must carry, marks api.rst done, and annotates the three CLUSTER_ENV_VARS references. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
docs/shared-deployment-pool-prompt.md is a personal instruction file
addressed to an agent ("Do the plan's steps 1-3. Stop before step 4 ...
that is mine to run, not yours."), not documentation. The plan it drove,
docs/shared-deployment-pool-plan.md, is checked in and annotated, so
nothing is lost.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every §1/§2 item in the review now carries a resolution line naming the commit. §1.4 and §2.2 are marked won't-do: whatsnew.rst is generated at release time by /bump-version, so the breaks are enumerated in the untwist plan's Part 7 for the release commit messages instead. Both §3 decisions are recorded, and the verification list is corrected where the review got it wrong -- step 7's `make -C docs html` names a Makefile that does not exist, and the real target starts a Docker container and overwrites the committed HTML, so api.rst ships unbuilt. Three places where the review's own claims needed adjusting are noted: the pool plan's bullet split, the Fusion command estimate having been exact at the time, and §1.5 having missed two more env reads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes the last unintentional v1 dependency in Fusion SQL. All seven EXPORT
handlers resolved their target with get_workspace_group({}), while
v2/export.py's ExportService.__init__ and _get_exports both take a Cluster --
which is why docs/versioned-management-api-review.md §3a left the
management/export.py repoint Open: the shim could not move until the grammar
did.
The grammar now moves. Each handler resolves with get_cluster({}) and imports
from management/v2/export.py directly, naming the version at the import line
for the same reason handlers/utils.py pins its managers: the egress routes
differ by version (clusters/{id}/egress/... against
workspaceGroups/{id}/egress/...), so these must not follow the
management.version option onto the other one. With nothing left on v1,
management/export.py repoints to .v2.export, documented as a version-locked
shim rather than a version-neutral one -- v1 takes a WorkspaceGroup and v2 a
Cluster, so the two do not fit behind one name.
Two consequences worth naming. The environment variable that identifies the
export target changes from SINGLESTOREDB_WORKSPACE_GROUP to
SINGLESTOREDB_WORKSPACE, since that is what get_cluster reads; all seven
handlers are hidden (_enabled = False), so this reaches nobody who has not set
SINGLESTOREDB_FUSION_ENABLE_HIDDEN. And SHOW EXPORTS now takes the ID from
ExportStatus.export_id rather than from its _info() body -- _info() is a
per-export status GET, which is not documented to echo egressID back, and
_get_exports already read the ID from the listing to build each object.
No IN CLUSTER clause was added: these commands took no target clause at v1
either, so adding one is a grammar change rather than part of the version move.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
v2 is the default everywhere else in the SDK, and handlers/cluster.py has covered the whole v1 workspace vocabulary since it landed, but both grammars were presented as equals: nothing told a user typing CREATE WORKSPACE GROUP that CREATE CLUSTER is where the SDK has moved. Adds SQLHandler._deprecated_by, naming the replacement command. When set, execute() warns once per execution -- after compile(), so command_key is populated and the message can name the command the user actually typed. This mirrors the existing _preview / PreviewFeatureWarning mechanism rather than inventing a second one. Set on all nine v1 workspace commands. SHOW REGIONS is the one exception: v2 assigns no region IDs, so SHOW CLUSTER REGIONS cannot report the ID column and is not a drop-in. Warning there would push callers who need that column toward something that lacks it. The reason is recorded at the handler, and a test asserts the exception stays exactly one command wide. The new warning is DeprecatedFeatureWarning(UserWarning), not the builtin DeprecationWarning, for the same reason PreviewFeatureWarning is a UserWarning: Python ignores DeprecationWarning outside __main__ by default, and these fire from library frames well below the notebook cell that triggered them, so a DeprecationWarning would reach almost nobody. manage_workspaces() keeps the builtin, where the caller's own frame is close enough for the default filter to behave. Nothing is removed and no grammar changed, so an existing v1 script keeps working -- it just says where to go. Four tests in TestFusion cover it, all token-free: the v1 commands each name a registered replacement, the v2 commands name none, the warning fires and names both commands through a probe handler, and an undeprecated command stays silent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
v2 is already the default everywhere: the management.version option, DEFAULT_VERSION, Manager.default_version, FilesManager.default_version, and the export shim all name it. What was missing was the other half -- marking v1 as deprecated -- and one place where flipping the default had gone further than deprecation and taken a v1 capability away. Mark v1 deprecated. _version_import._warn_if_deprecated_version() raises a DeprecationWarning whenever a public entry point *resolves* to v1, so it fires for an inherited management.version=v1 as much as for an explicit version='v1'. Wired into manage_files, manage_regions, and -- via _versioned_attr, their shared dispatch -- get_organization, get_secret and get_stage. Three deliberate exclusions: _resolve_version itself, so the v1-by-design internal paths (_manage_workspaces_v1, and the inference API behind it) stay silent rather than emitting noise the caller cannot act on; manage_workspaces, which keeps its own more specific message naming manage_clusters and so warns exactly once; and manage_clusters, which raises at v1 instead. Every module under v1/ now carries a .. deprecated:: note, and the ten classes v2 genuinely replaced name their replacement. The three modules that only re-export a shared implementation (files, region, billing_usage) mark the module *path* only -- a class-level note there would show up on the v2 class too. v1/inference_api.py is the one module deliberately left un-deprecated: inference/* has no v2 counterpart, so there is nowhere to send callers. Its docstring says so, and records the consequence -- it has to move somewhere version-neutral before management/v1/ can be deleted, or the inference API and the Fusion model commands go with it. Keep v1 working. Deprecated is not removed, and flipping the default may not take a v1 capability away. It had: because manage_workspaces() resolved management.version, and that now says v2, a bare call -- the overwhelmingly common one -- raised ManagementError. Pin it to v1 instead. It is the one public entry point the option does not steer, and rightly so: the option selects between implementations of a resource that exists at more than one version, and workspaces exist only at v1. An explicit version='v2' still raises. The asymmetry with manage_clusters(), which does consult the option and raises at v1, is intentional. It rests on what the option's value tells you now that it defaults to v2: reading 'v2' is no signal, since that is just the default, so it cannot justify refusing a workspace manager, while reading 'v1' is a signal -- nobody arrives at it without setting it -- so manage_clusters() is right to treat it as a deliberate request it cannot satisfy. Unpin the neutral notebook globals. notebook/_objects.py imported management.workspace, whose get_secret/get_stage/get_organization are re-exports of the *v1* implementations, so the notebook secrets, stage and organization globals ignored management.version entirely and ran on v1 whatever it said. They now come from the version-neutral package. Consequence recorded in the module: a v1 notebook environment has to set SINGLESTOREDB_MANAGEMENT_VERSION=v1, which is the cost of them being neutral at all -- pinned, v1 worked and v2 was simply broken. The workspace and workspacegroup globals stay on the shim, since v2 has no such resource and there is no cluster global to proxy to; that is a port, not a version bump. Tests: TestDeprecatedVersionWarning covers the warning on both routes to v1, the v2-is-silent half, and test_v1_still_works -- every entry point returns a working object on a real /v1/ route, and nothing raises because the default moved. TestV1IsDocumentedAsDeprecated enforces the docstring notes, including that the shared re-exports do *not* carry a class-level one. TestConfigOption's workspace test is inverted to assert the option does not reach manage_workspaces. Docs: api.rst, ADR 0001, the plan doc's user-visible-breaks list and the review doc all described the old raising behaviour and are corrected. The plan doc's Part 7 still had the export repoint open; 6f9d3a9 closed it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three unresolved review threads, all real defects, plus a stray debug
print and a grammar keyword rename.
visit_number read node.text, but the `number` rule is `<regex> ws*` and
`ws` matches `/* ... */` as well as whitespace, so a comment directly
after a numeric literal made float() raise on otherwise valid Fusion
statements. Read the regex child's text instead: unlike
flatten(visited_children)[0] it is not confused by the optional fraction
group, which matches empty for a bare integer.
UPLOAD CUSTOM MODEL built the remote path from the whole local_path, so
an absolute or nested path replayed the local directory tree into the
models space (model_name/tmp/weights.bin). Use the basename. The
directory branch was already correct -- upload_folder re-bases each
entry against local_root.
Both upload_folder implementations normalized their remote prefix
without strip_leading, unlike the listdir/download_folder call sites
that already passed it. FileSpace._upload builds
`files/fs/{location}/{path}`, so a leading '/' produced a doubled
slash; the `path = local_path` fallback could leak a './' prefix too.
Also drops the stray print(visited_children) from visit_compound, and
renames the CREATE CLUSTER scale-factor clause from WITH SCALE FACTOR to
USING SCALE FACTOR, with the rule name following the keyword as the rest
of that grammar does.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pyproject.toml's addopts sets -n 3, so every workflow step inherited xdist. The HTTP/Data API steps must not: they set SINGLESTOREDB_INIT_DB_URL, so load_sql's setup connection is MySQL and takes the `SET GLOBAL HTTP_PROXY_PORT` + `RESTART PROXY` branch (singlestoredb/tests/utils.py:227) once per worker, and a proxy restart drops whatever HTTP request another worker has in flight. Adds -n 0 to the HTTP steps in code-check.yml and coverage.yml. The https smoke-test step gets it too: it avoids the restart -- with no INIT_DB_URL its setup connection is itself HTTP, so that branch is skipped -- but it drives the same Data API, so it should not be the lone parallel run of it. The MySQL steps are unaffected: http_port stays 0 for a non-http URL, so the restart never happens and they keep the parallel default. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A cl-test-shared-0-* cluster was found ACTIVE and untracked after a run. Four separate holes let that happen; the first is silent, which is why it went unnoticed. 1. A creation call that fails while waiting left nothing tracked at all. Every creator brings the deployment into existence and only then waits for it -- create_cluster does its get_cluster before _wait_on_state (v2/cluster.py:1426) -- so a timeout, a transient error, or a Ctrl-C raises after the server already has a live, billable deployment. Since tracking wrapped only the return value, nothing registered it: no per-class sweep, no end-of-session sweep, and no summary line. The shared cluster pool is the worst-exposed caller, being the only one that waits with wait_on_active=True and wait_timeout=1200. _CREATORS now carries a finder per creator, and the wrapper looks the orphan up by name and tracks it when the call raises. BaseException, not Exception, so an interrupt mid-wait reaps too. 2. cleanup_tracked dropped every entry before trying to terminate it, and only logged a failure -- so one transient error leaked the deployment permanently, with no retry and no mention in the summary. Entries now stay tracked until they are confirmed gone or actually terminated. 3. _is_gone treated any refresh failure as "already gone", which is right for a 404 and wrong for a 503: it skipped the termination and left the cluster running. Only a 404 counts as gone now; anything else reports still-live, since a redundant terminate costs one round trip and a missed one costs money. 4. Both the sweep and the container cleanup lived only in pytest_unconfigure, which a cancelled CI job or a killed xdist worker never reaches. Adds atexit and SIGTERM fallbacks -- verified to fire on both an unhandled exception and a signal, preserving exit code 143. SIGKILL stays unreachable; cleanup_deployments.py is the net for that. conftest also now reports anything still tracked after the final sweep, naming cleanup_deployments.py, so a leak is loud instead of silent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit aae8466. Configure here.
Bugbot found both in the leak-prevention machinery added by aae8466, and each one can leave a real, billable cluster running. `_tracking_wrapper` guarded orphan recovery with `_is_mocked(receiver)`, but that helper looks for a `_manager` attribute and the receiver here is the manager, which has none -- so a unit test driving a real manager with a patched `_post` read as live and the recovery fired an actual management API GET. `_creator_is_mocked` is the helper that inspects the receiver's own transport, and it already handles both receiver shapes. The out-of-band sweeps (SIGTERM, atexit, unconfigure) only walked `_tracked`, which a create that has POSTed and is blocked in `wait_on_active` has not entered yet: the wrapper tracks on return and recovers in its `except`, and a killed process runs neither. So creations are now listed in `_in_flight` for their duration, and a whole-session sweep drains that list through the same `_recover_orphan` first. Whoever claims an entry -- the sweep or the wrapper's `except` -- is the one that recovers it, so nothing gets tracked twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Summary
mgr.v2/entity.v2attribute syntaxmanagement/v1/andmanagement/v2/folders; top-level modules become thin re-export shims routing viaconfig.get_option('management.version')VersionedMixinproviding cached__getattr__-based version switching for both managers and entities, with credential cloning andfrom_dictreconstructionTest plan
test_versioned_management.pycovering:__getattr__pattern matching and cachingfrom_dict+ versioned managermanage_*()version routingManagementError)management.versionconfig option routing🤖 Generated with Claude Code
Note
High Risk
Flips the default management API to v2, changes cloud deployment/provisioning behavior and Fusion routing, and runs parallel management tests that provision real clusters—mistakes can cause billing leaks or broken HTTP/MySQL test runs.
Overview
Makes Management API v2 the default and replaces the old cross-version bridge (
VersionedMixin,.v1/.v2switching,_responsestorage) with version namespaces (management/v1/,management/v2/) and shared implementations level-set to v2, routed through_version_importandmanagement.version(now defaultv2).Public surface:
manage_clustersis exported as the primary entry point; version-neutral helpers (get_organization,get_secret,get_stage) dispatch by resolved version;manage_workspacesstays pinned to v1 with deprecation warnings. Legacymanage_clusteris removed from package exports. v1-only paths (Fusion workspace grammar, inference AI helpers,stage://UDF uploads) call_manage_workspaces_v1so they do not warn or follow the new default.Fusion: Adds v2 CLUSTER commands (
SHOW CLUSTERS,CREATE CLUSTER, etc.) viaget_cluster_manager(); repoints deployment resolution, files, jobs, and export handlers toward v2 where safe; marks v1 workspace handlers deprecated at runtime. v2 cluster create/update gains firewall polling sowait_on_activeis closer to “usable,” plus related test and audit doc updates.Tests & CI: Default
pytest -n 3 --dist loadgroup(shared cluster pool +management_v1marker); HTTP/Data API jobs force-n 0to avoid proxy restart races; wheel smoke tests add pytest-xdist; conftest adds deployment tracking/sweep, SIGTERM/atexit cleanup, and optional management API timing summaries (SINGLESTOREDB_MANAGEMENT_TRACE).Docs/config: ADR 0001 and implementation plans, expanded
api.rstfor clusters, flake8 ignores for version re-export modules, and test resource scripts pinversion='v1'.Reviewed by Cursor Bugbot for commit 666d960. Bugbot is set up for automated code reviews on this repo. Configure here.