Add versioned management API wrappers - #126
Open
kesmit13 wants to merge 24 commits into
Open
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Fix import error masking in _import_versioned_module by validating version format and only catching ModuleNotFoundError for the expected path. Add None guards on _manager in entity/wrapper paths. Fix docstring copy/paste error, clarify ADR re-export behavior, remove dead .flake8 entry. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Route entity and wrapper-manager version switches through getattr(self._manager, version) instead of calling _get_versioned directly, so they share the same cached versioned manager instance that mgr.v2 returns. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ports - Replace fromisoformat() with to_datetime_strict() in UsageItem.from_dict() - Fix snake_case key access (resource_type -> resourceType, Usage -> usage) - Propagate _location and region after entity version-switch reconstruction - Return List[ExportStatus] from _get_exports() instead of raw JSON - Fix region.py module docstring and manage_files docstring Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When cloning a Manager via version switching (e.g., mgr.v2), the explicit access_token argument caused _is_jwt to evaluate to False in the clone, preventing JWT refresh on subsequent requests. Propagate _is_jwt from the original manager to preserve token refresh behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…cstring Shallow-copy _location and rebind its _manager to the versioned manager so version-switched entities don't leak calls back to the original API version. Also corrects InferenceAPIInfo.from_dict's return docstring, which incorrectly referenced :class:`Job`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Wires in the first round of audit-driven field/method coverage on the versioned management API and adds 31 mock-only tests (12 new classes) covering both the staged additions and plumbing regressions. Source changes (audit-driven, already staged): - Workspace: auto_scale, kai_enabled, scale_factor; cache_config widened to float; Workspace.update accepts the three new kwargs - WorkspaceGroup: deployment_type, expires_at, high_availability_two_zones, opt_in_preview_feature, outbound_allow_list, project_id, project_name, smart_dr_status, state, update_window, provider, region_name; new WorkspaceGroup.update kwarg deployment_type; WorkspaceManager.create_- workspace_group accepts provider/region_name/deployment_type/HA/preview/ project_id - JobsManager.schedule: max_allowed_execution_duration_in_minutes injected into executionConfig - Secret.from_dict: createdAt, lastUpdatedAt, deletedAt parsed via to_datetime; signature widened to accept Optional - v2/region.py: RegionManager subclass overriding list_regions to hit /v2/regions; list_shared_tier_regions raises (no v2 counterpart) - v2/workspace.py: WorkspaceGroup subclass with get_metrics() returning raw OpenMetrics text; resolves org id from manager._organization_id -> _params['organizationID'] -> manager.organization.id Test additions (singlestoredb/tests/test_versioned_management.py): - Plumbing regressions: TestLocationManagerRebind, TestJWTRefreshInClones, TestDateTimeParsingFixes, TestEntityRoundTripFidelity - Field coverage: TestWorkspaceFromDictNewFields, TestWorkspaceUpdate- Posting, TestWorkspaceGroupNewFields, TestWorkspaceGroupCreateUpdate- Posting, TestJobsManagerScheduleDuration, TestSecretFromDictTimestamps - v2 behavior: TestV2RegionBehavior, TestV2WorkspaceGroupGetMetrics - Routing: TestManageRoutingForAllFactories iterates manage_workspaces, manage_regions, manage_files All tests are mock-only (no Docker, no SINGLESTOREDB_MANAGEMENT_TOKEN); 68 tests pass in <1s. The existing live suite (test_management.py) is unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
v2 region listings have no regionID, so v2 Region instances carry id=None. The previous lookup matched only on id, falling back to a '<unknown>' placeholder for every workspace group on a v2 manager. Now match by id first, then by (regionName, provider), and use payload fields for the final fallback so users see real region info even when no listing match exists. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Addresses Copilot PR review comments 3365002592 and 3365002611 on PR #126. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- v2/workspace.py: fall back to self._manager.v1.organization.id in WorkspaceGroup.get_metrics. v2 has no organizations/current endpoint and the OpenAPI spec for the v2 metrics endpoint explicitly directs callers to /v1/organizations/current. Docstring updated to make this cross-version exception explicit. - versioned.py: _import_versioned_module now distinguishes between an unsupported API version (the version package itself is missing) and a missing submodule under a valid version. Unrelated ModuleNotFound errors (e.g., transitive deps inside a valid module) propagate untouched instead of being masked. - test_versioned_management.py: updated tests to assert the new submodule-missing message and rewired metrics fallback tests to stub the v1 clone's organization via _version_cache. Addresses Copilot PR review comments 3375344342 and 3375344393 on PR #126. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Add path-traversal containment check in recursive folder downloads. New `singlestoredb.management.utils.ensure_within` resolves both the destination root and the candidate target via realpath and raises ManagementError if the target escapes the root. Applied in `FileLocation.download_folder` (file and directory branches) and `Stage.download_folder`. Defends against `../` segments and symlink escapes from a malicious or compromised remote listing. - Route the v1-namespace `manage_regions`, `manage_workspaces`, and `manage_files` factories through `_import_versioned_module` so that `version='v2'` returns the correct v2 manager class instead of a v1 manager pointed at a `/v2/` base URL. - Convert `singlestoredb/management/v1/inference_api.py` imports from absolute (`from singlestoredb.*`) to the relative form used by the rest of the v1 modules. - Fix over-indented continuation lines on two `get_executions` signatures in `v1/job.py`. - Rename `self` to `cls` (and use `cls(...)` for construction) in `ExportService.from_export_id`, which is a `@classmethod`. - Add tests for path-traversal rejection in both download_folder methods and for v1-namespace factory routing to v2. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Stage.upload_folder ignored stage_path and built remote targets from the
local filesystem path (with os.getcwd() basename when include_root=True),
and crashed on subdirectories because glob('**') yields directories.
FileSpace.upload_folder used str.lstrip(local_path), which strips a
character set rather than a prefix, producing incorrect remote paths.
Both now walk files via os.walk, compute the per-file suffix with
os.path.relpath, and honor include_root and recursive=False consistently.
…nager The v2 RegionManager raised a custom ManagementError when callers used list_shared_tier_regions, on the grounds that /v2/regions/sharedtier has no OpenAPI counterpart. Every other v1 endpoint without a v2 counterpart just 404s through the inherited request path, so singling out this one method gave a misleading impression of v2 endpoint coverage. Drop the override, the related docstring paragraph, the now-unused ManagementError import, and the test that asserted the raise — falling back to the same 404 path used by the rest of the v2 surface. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
v2 Region objects have id=None and identify by (provider, region_name). When such a Region was passed to WorkspaceManager.create_workspace_group, the code left it in the JSON body as regionID and never populated the provider/regionName fields. Now, when region.id is falsy, unwrap the Region into provider/region_name (without overriding explicit kwargs) and send regionID as None. v1 Regions with an id continue to work as before. Reported by Cursor Bugbot on PR #126. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Workspace.refresh, StarterWorkspace.refresh, and WorkspaceGroup.refresh were passing every Mapping-typed attribute through snake_to_camel_dict / camel_to_snake_dict. Because new_obj is produced by from_dict, _response already holds the raw camelCase API dict and public fields like auto_scale / auto_suspend are already snake_cased — the conversion lowercased _response keys (breaking later entity.v2 version switches) and re-camelCased user-visible fields. Also clear _version_cache in refresh(), so accessing entity.v2 after refresh rebuilds the clone from the refreshed _response instead of returning the stale pre-refresh cache. Reported by Cursor Bugbot on PR #126. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Stage.download_folder and FilesManager.download_folder pass the result
of ensure_within() through os.path.dirname() before os.makedirs().
ensure_within returns os.path.normpath(...), which collapses './foo.txt'
to 'foo.txt', so os.path.dirname('foo.txt') is '' and os.makedirs('')
raises FileNotFoundError. This hits the default local_path='.' case
whenever overwrite=True lets execution past the existence pre-check.
Fall back to '.' when the dirname is empty. ensure_within is unchanged.
Reported by Cursor Bugbot on PR #126.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
kesmit13
force-pushed
the
versioned-management-api
branch
from
August 20, 2026 12:59
6b049fd to
bb4ab24
Compare
upload_folder compares each walked file path against the set of paths expanded from the `ignore` glob patterns. The walk side is built from os.path.normpath(local_path), but the glob side was left as-is, so a non-normalized pattern (e.g. './dir/*.log' for local_path './dir') produced './dir/a.log' vs 'mydir/a.log' and the ignore silently had no effect. Normalize the glob results so both sides are comparable. Also correct ADR 0001, which referred to an `api_version` class attribute; the implementation uses `default_version`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses two review findings on the v1 management folder helpers: - Stage.download_folder passed listdir-relative entry names straight to is_dir/download_file, so nested downloads requested the wrong remote objects. The stage_path prefix is now normalized and rejoined onto each entry, matching FileSpace.download_folder. The ensure_within destination check also moved ahead of the remote calls so traversal entries fail before any request is made. - upload_folder expanded ignore globs from the process working directory but compared them against os.walk paths rooted at local_path, so documented patterns like '**/*.pyc' never matched. Glob expansion is now the shared resolve_ignore_files() helper in management/utils.py, used by both Stage.upload_folder and FileSpace.upload_folder. Relative patterns resolve against local_path and globbing is always recursive so '**' works regardless of the recursive upload flag. Adds regression tests for both, each of which fails on the prior code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
resolve_ignore_files() returns os.path.normpath'd glob results, but both upload_folder implementations tested raw os.walk paths for membership. With local_path='.', os.walk yields './sub/skip.pyc' while the glob side normalizes to 'sub/skip.pyc', so ignore patterns silently failed and excluded files were uploaded anyway. Normalize the walk-side path so both sides use the same form, and note the requirement in the resolve_ignore_files docstring. Adds cwd-relative regression tests for Stage and FileSpace. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Findings from a review pass over the four folder helpers: - Folder ignore patterns silently did nothing. Glob expansion returns matched directories, but membership was only tested against file paths and os.walk was never pruned, so 'ignore="**/__pycache__"' still uploaded every file inside. Both upload_folder implementations now prune walked directories against ignore_files, and the docstrings say folders are supported. - Remote paths were built with os.path.join, so a Windows client would create stage/file objects named 'dest\sub\b.txt' on a Linux server. Remote paths are now always '/'-joined in both upload_folder implementations and in FileSpace.download_folder. - Stage.download_folder issued an is_dir (and therefore an info GET) for every listing entry, and a second one inside download_file, where FileSpace gets the type from the listing for free. Stage now lists with return_objects=True and uses entry.type, and delegates to a new Stage._download_file(..., _skip_dir_check=True), mirroring FileSpace. Empty remote folders are now created locally, as FileSpace already did. Public download_file behavior is unchanged; it delegates to _download_file with the directory check enabled. 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 1 potential issue.
❌ 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 f30d315. Configure here.
local_path is the destination folder that download_folder creates, and
the existence guard exists to protect it (see the DOWNLOAD CUSTOM MODEL
... TO 'name' OVERWRITE fusion surface). The default of '.' contradicted
that: the current directory always exists, so download_folder('remote')
raised OSError unconditionally and the default only worked with
overwrite=True, where it dumped contents straight into the cwd.
local_path now defaults to None, meaning "the remote folder's name in the
current directory", so download_folder('data/models') creates ./models/.
Downloading the root folder with no local_path raises ValueError rather
than deriving an empty name.
Compatibility: any explicit local_path behaves exactly as before,
including the fusion models handler, which always passes one. Omitting
local_path previously raised (overwrite=False) or crashed with TypeError
(explicit None), so nothing could depend on those. The one real change is
download_folder(x, overwrite=True) with no local_path, which used to
overwrite files in the working directory and now creates ./x/.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cursor Bugbot on PR #126 flagged that the remote-prefix handling in the folder helpers only stripped trailing '/', so a prefix built with os.path.join (as the fusion CUSTOM MODEL handlers did) kept a trailing '\' on Windows and produced malformed remote paths like 'llama3\/file'. Add normalize_remote_path() to management/utils.py, which converts '\' to '/', collapses duplicate separators, strips the trailing separator, and optionally strips leading './' and '/'. Apply it to every caller-supplied remote prefix in FileSpace and Stage (upload_folder, download_folder, listdir), and use the normalized prefix for the info/exists/is_dir/listdir calls those helpers make so a Windows-style argument can't reach the API unnormalized. Also fix the root cause in the fusion UPLOAD / DOWNLOAD / DROP CUSTOM MODEL handlers, which were assembling remote paths with os.path.join. Add offline unit tests in TestRemotePathUtils. 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
Medium Risk
Large structural refactor of management client code plus removal of
manage_cluster; version switching and file-download path checks affect API behavior and security-sensitive file I/O.Overview
Introduces a versioned management API layer (ADR 0001): implementations live under
management/v1/andmanagement/v2/, while top-level modules stay as v1 re-export shims.manage_files,manage_regions, and similar factories now pick the API version from theversionargument ormanagement.versionconfig via dynamic imports.VersionedMixinonManagerand entity types enables cached switching withmgr.v2/entity.v2—managers clone credentials to the new base URL; entities rebuild from stored_responsewithout another fetch. v2 modules mostly re-export v1;RegionManageris an early v2 override for the different regions response shape.Breaking: deprecated cluster management is removed (
cluster.py,manage_clusterfrom package exports). Compatibility fixes while moving code to v1 include billing usage field mapping, export status list parsing, andensure_withinon folder downloads to block path traversal.Existing import paths for workspace/files/etc. unchanged at the shim layer; callers that used
manage_clustermust use workspaces instead.Reviewed by Cursor Bugbot for commit 6b049fd. Bugbot is set up for automated code reviews on this repo. Configure here.