Add versioned management API wrappers - #126
Open
kesmit13 wants to merge 83 commits into
Open
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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>
The UDF stage:// handling and the Fusion cluster handlers went through
_manage_workspaces_v1() and spoke workspace-group vocabulary, so they
only worked against v1 even when the caller's deployment was a cluster.
- functions/ext/{asgi,mmap}.py: resolve stage through the neutral
get_stage(hostname) instead of a v1 workspace manager, and translate
its RuntimeError into a ValueError that names what is missing.
- fusion/handlers/utils.py: add _deployment_param() plus the shared
_DEPLOYMENT_KEYS/_GROUP_SPELLING_HINT, so a handler can accept either
spelling and hint at the other one when lookup fails.
- fusion/handlers/cluster.py: resolve regions off the manager's
TTL-cached regions list, matching case-insensitively.
- management/v1/, management/v2/: docstrings document the classes and
functions rather than narrating the v1-to-v2 migration.
- tests/test_fusion.py: cover the deployment-key handling and hints.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The default version was spelled 'v2' as a literal in the config option registration, in Manager.default_version, in FilesManager, and in DEFAULT_CLUSTER_VERSION, so retargeting the SDK at a new version meant finding all of them and hoping the option and the classes stayed in step. Add singlestoredb/_management_version.py holding DEFAULT_MANAGEMENT_VERSION and DEPRECATED_MANAGEMENT_VERSION. It imports nothing, which is what lets both config.py and management/ read it: config is imported before management, and management.manager imports config, so neither package can host the constants. Every literal that meant "the current default" now comes from there -- the option default, _version_import.DEFAULT_VERSION, Manager, and DEFAULT_CLUSTER_VERSION. FilesManager's override is deleted outright; it only ever restated Manager's value. Literals stay where the point is a specific version rather than the current one: the default_version of a version-specific class, and the v1 guards in manage_workspaces() and manage_clusters(). default_version deliberately still does not read the option. Nor get_default(): Option.__init__ folds the environment variable into the registered default, so a class reading it would take a v1 URL from SINGLESTOREDB_MANAGEMENT_VERSION=v1. The new subprocess test covers that hole, which no in-process set_option() can reach. Also: docstrings across the management modules document the code rather than narrating the v1-to-v2 migration or carrying dated "verified live" claims, _manage_workspaces_v1's error no longer advises setting an option it never reads, and ADR 0001 plus the two plan documents record the constant and drop claims that are no longer true. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
It changes default management API behavior (v2-by-default), restructures many public entry points, and updates CI/test execution semantics, making the regression surface too broad for automated approval.
Review details
Suppressed comments (1)
singlestoredb/notebook/_objects.py:208
- Same late-bound
attrissue as inStage: this annotations loop usesattrfrom the priordir(_OrganizationBase)loop when callingfunctools.update_wrapper, so wrapper metadata comes from an unrelated member and depends on loop ordering. Consider returningproperty(wrap)here (or wrapping against a resolved member form) to keep behavior stable and intent clear.
- Files reviewed: 73/78 changed files
- Comments generated: 1
- Review effort level: Lite
The four notebook proxy classes build their properties in two passes: one over dir() of the class they front, one over its __annotations__. The second pass ran functools.update_wrapper(wrap, attr) against `attr`, which only the *first* pass ever assigns -- so every annotation-backed property took its __name__, __doc__ and __wrapped__ from whichever public dir() member happened to be last. There is no attribute object behind an annotation to copy metadata from, so there was never a right value for that call to find; had a fronted class carried annotations but no public dir() members, `attr` would have been unbound and __new__ would have raised NameError. Return property(wrap) instead, and drop the is_method parameter these four factories accepted and never read. Renaming them to make_annotation_wrapper keeps mypy from reading the pair in each __new__ as one conditionally redefined function, and says which pass is which. The dir() loops keep update_wrapper: there `attr` is the member being proxied, and copying its signature and docstring is the point. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A deployment is named the same way whatever kind it is, so a qualified IN CLUSTER resolved exactly where the bare IN already did -- get_deployment() treats every spelling as one code path. The clause was six copies of a four-line grammar block buying nothing, plus a first-match ordering hazard: let in_cluster fall below in_deployment and IN CLUSTER 'x' silently parses as a deployment named CLUSTER. The six handlers now take a bare IN, with IN GROUP kept only because it already parses. IN CLUSTER never reached main, so there is nothing to deprecate. The v1 WORKSPACE grammar is untouched: it is on its way out, and giving it a new spelling would change behaviour there for no gain. Also retarget the two messages that pointed users at IN CLUSTER -- the IN GROUP synonym hint and the SINGLESTOREDB_WORKSPACE_GROUP error. The grammar test inverts to asserting the spelling is absent and that SHOW STAGE FILES IN CLUSTER 'c1' fails to parse. That last assertion is the point: the risk in removing an alternation branch is the spelling reinterpreting as a deployment named CLUSTER rather than erroring. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It was the one command in handlers/workspace.py left without a _deprecated_by pointer, on the grounds that SHOW CLUSTER REGIONS drops the ID column and so is not a drop-in. But the reason to warn is the route, not the columns: this command reads the v1 API, and that is what is going away. A caller holding a v1 region ID needs to hear it now rather than when the route stops answering. Point it at SHOW CLUSTER REGIONS and record the column difference where a reader will meet it -- a Remark on SHOW REGIONS itself, next to the ID column it is about. The test asserted the exemption, so it asserted the opposite of what we now want; it now requires that nothing in the module is undeprecated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
WITH PROVIDER reads as though it sets something on the cluster, which it does not: it narrows which region IN REGION means when two providers offer a region under the same name. USING says that -- and matches USING SCALE FACTOR, the other clause in this grammar that qualifies a value rather than setting one. Renames the rule as well as the keywords, so the params key follows, in both handlers that take the clause: CREATE CLUSTER, where it is optional, and CREATE STARTER CLUSTER, where both halves of the region are required. The old spelling is not accepted -- this grammar has not shipped, so there is nothing to keep working. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
That variable names a project of the inference API, not of the cluster
management API. The two are separate namespaces and the notebook
environment reports different IDs for them: a notebook attached to a
cluster in Standard Project publishes an ID that
GET /v2/projects/{id} answers 404 project not found for. Reading it in
_resolve_project_id() therefore broke CREATE CLUSTER in every notebook,
in every organization -- the failure was a hard KeyError, since an
environment-derived ID was treated as authoritative.
Priority two becomes the project of the deployment the code is running
in, read off SINGLESTOREDB_WORKSPACE via GET /v2/clusters/{id}. That is
a better default than the variable ever was: a new cluster lands beside
the one it was created from, which makes IN PROJECT optional in a
notebook even in an organization with several projects. A deployment
that cannot be read -- outside a notebook, a starter cluster, a stale ID
-- falls through silently, because there are further defaults to try.
get_project_id() stays for its one legitimate caller, inference_api.py,
and is now where the distinction is documented. The Fusion get_project()
resolves the IN PROJECT clause and nothing else. The test suites used the
same variable to pick a deployment target, which was never its meaning
either; that override is now SINGLESTOREDB_TEST_PROJECT.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Uploading one file made two identical exists() requests -- upload_file checked, then _upload checked again -- and ended with an info() request that all three Fusion upload handlers threw away. The check now lives only in _upload, and _upload takes a private fetch_info flag so a caller that discards the FilesObject does not pay for building one. Both upload_file methods delegate to a shared _upload_local_file on FileLocation, which is also what the Fusion handlers call; it wraps the local open() in a with block, since the conflict is now raised after the handle exists. A Stage or file space upload drops from four requests to three, from six to five with overwrite, and a folder upload saves one per file. The public upload_file still returns a populated FilesObject. CountingManager in tests/utils.py is the harness for this: it stands in for a Manager, simulates the filesystem, and records every request, so the round-trip count of an operation is assertable without a deployment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
test_create_drop_workspace_group named its subject
'Create WG Test {id(self)}', which matches nothing in
cleanup_deployments.PATTERNS. So every run that died between the create
and the drop -- or whose terminate failed, which that test swallowed
silently -- left a live workspace group that the maintenance sweep
reported as "No leftover test deployments found". They accumulate until
someone reaps them by hand through the management API.
The name is now a random hex token, which id(self) never was: an address
repeats across processes, so two workers could pick the same one. The
pattern added for it accepts hex, and decimal is a subset, so the groups
older runs stranded are reaped by the same sweep.
The general fix is --show-unmatched: find_leftovers now also returns the
live deployments whose names it does not recognize, so the next test that
invents a name is visible instead of silently filling the organization.
The two parse-failure cluster names get an id suffix for the same reason,
and the test's own cleanup reports a failed terminate rather than
discarding it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
--show-unmatched turned up seven stranded workspace groups named 'Stage Fusion Testing <n> <id>' and 'Files Fusion Testing <id>', 64 to 232 hours old. Those names were retired on this branch -- TestStageFusion moved to v2 clusters and then to the shared pool, and TestFilesFusion stopped creating a group it never read -- but they are still live in the organization and still billing. They also keep arriving, and this is the reason strays recur: none of the cleanup machinery is on main. No utils.track(), no per-class sweep, no cleanup_deployments.py -- a run there has only tearDownClass, so a killed run or a setUpClass that raises leaks a group permanently, under the old names. LEGACY_PATTERNS matches them until main carries the sweep. Not matched, on purpose: groups named 'Group <hex8>'. No revision of this repo generates that, so a pattern for it would be a guess with a live workspace group on the other end. --show-unmatched will keep reporting it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
No revision of this repo generates these names, so attribution cannot justify the pattern; it is here because the deployments are in the organization and are being billed. The eight-character floor is the guard: 'Group 1' and 'Group 2' are what a person or the portal produces, and a bare [0-9a-f]+ would reap them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
80854d8 renamed the clause but only caught the sites spelled in upper case; this one builds the statement in lower case from an f-string, so it still said 'with provider' and the grammar rejected it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Stage 2 of docs/stage-upload-round-trips-plan.md: Cluster.project and StarterCluster.project resolve their reported projectID on first read rather than in from_dict, so resolving a deployment by name no longer drags GET /v2/projects behind the cluster listing it needs. The one-hour ttl_property on ClusterManager.projects still matters -- SHOW CLUSTERS EXTENDED reads .project per row and one fetch serves them all. Stage 1c: Stage._upload and FileSpace._upload fetched the same metadata twice, once through exists() and again through remove()'s is_dir(). They fetch it once now, through the shared FileLocation._info_or_none, and branch on the object; remove() keeps its own is_dir() for its other callers. An UPLOAD FILE TO STAGE ... IN '<name>' costs three requests instead of four, or four instead of six with OVERWRITE. The request-count harness grew a CountingClusterManager and run_fusion_statement so the count of a whole statement is pinned, not just the count of a Stage call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cluster.region was the one eager resolve Stage 2 left behind: a payload carrying a region made from_dict match it against ClusterManager.regions, so a realistic cluster listing paid a GET /v2/regions that an upload never looked at. It now stores the reported name and resolves on first read, falling back to a Region built from what the cluster itself reported -- the same shape as the lazy project. Neither lazy value is in vars(cluster), so vars_to_str would drop both from str(cluster), and resolving them in __repr__ would make printing a cluster issue two requests. vars_to_str takes an extra= mapping for this; Cluster.__str__ passes the resolved object when something has already read the property and the reported ID / name otherwise, so printing stays free. SHOW CLUSTERS EXTENDED and SHOW STARTER CLUSTERS EXTENDED report ProjectName rather than ProjectID, which is what the plan said all along. _project_from_id's '<unknown>' fallback means the name is always populated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The creation wrapper worked out from its receiver that a call was going through a mocked manager, used that to skip orphan recovery, and then handed the result to track() anyway. track() judges only what it is given, and is deliberately biased toward "real" for anything it cannot place -- a cluster left running bills money, a redundant terminate costs one round trip -- so a stubbed get_cluster returning a Cluster with _manager=None, or a bare 'sentinel' string, registered as a live deployment. The unit tests then ended with nine phantom deployments in _tracked, a failed refresh and a failed terminate logged for each, and a "9 deployment(s) left live" banner. Nothing had leaked. The cost is that the banner is the only thing that reports a genuinely leaked cluster, and constant phantom noise is how it stops being read. The receiver's verdict now decides whether the return value is tracked too. It is the authoritative one: nothing a mocked creator returns names a deployment that exists. 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 cluster provisioning/wait semantics and Fusion routing, and increases parallel live management tests—any mistake can affect billable cloud resources or break v1/v2 callers.
Overview
Replaces the earlier cross-version bridge (
.v1/.v2switching,VersionedMixin) with version namespaces (management/v1/,management/v2/), shared implementations level-set to v2, and routing through_resolve_version/manage_*factories.management.versiondefaults tov2viasinglestoredb._management_version;manage_clustersis the primary entry point whilemanage_workspacesstays pinned to v1 with deprecation warnings.Adds Fusion v2 cluster SQL (
SHOW/CREATE/DROP/USEcluster, projects, regions) and moves stage, files, jobs, and deployment resolution onto v2 managers where safe; v1 workspace grammar remains for compatibility. v2 cluster lifecycle gains firewall polling on create/update, lazyCluster.project/region, and documented handling of generated admin passwords.Tests and CI: splits management tests into v1/v2/versioning suites, introduces a shared cluster pool with
xdist_groupmarks, enables pytest-xdist (-n 3 --dist loadgroup) by default, and forces serial HTTP/Data API runs (-n 0) to avoid proxy restart races. Stage/files upload paths shed redundant HTTP round trips (per landed plan docs).Docs and tooling: ADR 0001, expanded
api.rstfor clusters, management API audit updates, flake8 ignores for version re-export modules,management.traceoption, and callers (notebook globals, UDFstage://, AI helpers) updated for version-neutral or v1-pinned paths.manage_cluster(singular) is removed from public exports.Reviewed by Cursor Bugbot for commit 5b750bf. Bugbot is set up for automated code reviews on this repo. Configure here.