From 2498f6bd92df1a4e7c90b449ebbdb4ed66c9935f Mon Sep 17 00:00:00 2001 From: Kevin Smith Date: Thu, 1 Oct 2026 15:09:11 -0400 Subject: [PATCH] Narrow v1 deprecation warnings to workspace creation The v1 deprecation warnings fired on every WORKSPACE command and on every entry point that resolved to v1, telling owners of existing workspace groups to switch to clusters when nothing requires it. Keep warnings only where they steer a choice the caller is making now: manage_workspaces(), and the Fusion CREATE WORKSPACE GROUP and CREATE WORKSPACE commands, whose message now says new deployments should be clusters. Remove the rest: the other Fusion WORKSPACE commands and version-less SHOW REGIONS, the IN GROUP clause on Stage commands, and the version='v1' warning on manage_files, manage_regions, get_organization, get_secret and get_stage. Co-Authored-By: Claude Opus 5.5 --- README.md | 9 +-- docs/src/api.rst | 26 ++++----- singlestoredb/_management_version.py | 5 -- singlestoredb/fusion/README.md | 2 +- singlestoredb/fusion/handler.py | 13 +++-- singlestoredb/fusion/handlers/stage.py | 31 +++------- singlestoredb/fusion/handlers/utils.py | 22 +------ singlestoredb/fusion/handlers/workspace.py | 33 ++--------- singlestoredb/management/_version_import.py | 55 ------------------ singlestoredb/management/files.py | 5 +- singlestoredb/management/region.py | 5 +- singlestoredb/tests/test_fusion.py | 54 ++++++++++-------- .../tests/test_management_versioning.py | 57 ++++++++----------- 13 files changed, 97 insertions(+), 220 deletions(-) diff --git a/README.md b/README.md index c2b617867..f645d481c 100644 --- a/README.md +++ b/README.md @@ -197,8 +197,9 @@ c = manager.create_cluster( The API is versioned, and version 2 — the flat `Cluster` resource shown above — is the default. Version 1, which called a deployment a `Workspace` inside a -`WorkspaceGroup` and is reached through `s2.manage_workspaces()`, still works -but is deprecated in its entirety. Select a version with the +`WorkspaceGroup` and is reached through `s2.manage_workspaces()`, still works. +`manage_workspaces()` is deprecated, because new deployments should be +clusters. Select a version with the `management.version` option (`SINGLESTOREDB_MANAGEMENT_VERSION`) or by passing `version=` to any `manage_*` function. @@ -258,8 +259,8 @@ conn.execute(""" ``` The `WORKSPACE` and `WORKSPACE GROUP` commands, and the version-less -`SHOW REGIONS`, still work but are deprecated along with the rest of management -API v1. +`SHOW REGIONS`, still work. `CREATE WORKSPACE GROUP` and `CREATE WORKSPACE` are +deprecated, because new deployments should be clusters: use `CREATE CLUSTER`. See [singlestoredb/fusion/README.md](singlestoredb/fusion/README.md) for details on writing custom Fusion SQL handlers. diff --git a/docs/src/api.rst b/docs/src/api.rst index 46fc2de23..27112fe15 100644 --- a/docs/src/api.rst +++ b/docs/src/api.rst @@ -340,18 +340,19 @@ with exactly one project does not need to name it. Workspaces (v1) ............... -.. deprecated:: Management API v1 as a whole is deprecated, not just the - workspace vocabulary below. ``management.version`` now defaults to ``'v2'``, - and every entry point that resolves to v1 raises a - :class:`DeprecationWarning` -- whether v1 was named with ``version='v1'`` or +.. deprecated:: :func:`manage_workspaces` is deprecated, because new + deployments should be clusters, and raises a :class:`DeprecationWarning`. + So do the Fusion SQL ``CREATE WORKSPACE GROUP`` and ``CREATE WORKSPACE`` + commands, which point at ``CREATE CLUSTER``. ``management.version`` now + defaults to ``'v2'``. + + **v1 still works.** Every function and class below still operates against + the live v1 endpoints, and :func:`manage_workspaces` still returns a working + :class:`WorkspaceManager` without being asked for a version. Other entry + points that resolve to v1 -- whether v1 was named with ``version='v1'`` or inherited from the ``management.version`` option - (``SINGLESTOREDB_MANAGEMENT_VERSION``). - - **v1 still works.** Deprecated here means warned about, not removed: every - function and class below still operates against the live v1 endpoints, and - :func:`manage_workspaces` still returns a working - :class:`WorkspaceManager` without being asked for a version. Nothing raises - because the default moved. When you are ready to move off v1: + (``SINGLESTOREDB_MANAGEMENT_VERSION``) -- do not warn. When you are ready to + move to clusters: ============================== ============================== v1 v2 @@ -369,8 +370,7 @@ Workspaces (v1) organizational unit rather than a deployment parent. :func:`manage_files` and :func:`manage_regions` need no migration -- their - routes are identical at both versions, so simply stop passing - ``version='v1'``. + routes are identical at both versions. The :func:`manage_workspaces` function will return a :class:`WorkspaceManager` object that can be used to interact with version 1 of the Management API. diff --git a/singlestoredb/_management_version.py b/singlestoredb/_management_version.py index 4425ca418..528575349 100644 --- a/singlestoredb/_management_version.py +++ b/singlestoredb/_management_version.py @@ -14,8 +14,3 @@ #: class. Classes that implement one specific version name it literally #: instead, and do not follow this. DEFAULT_MANAGEMENT_VERSION = 'v2' - -#: Management API version being wound down. Public entry points that resolve -#: to it raise a :class:`DeprecationWarning`, and everything under -#: ``singlestoredb.management.v1`` goes away with it. -DEPRECATED_MANAGEMENT_VERSION = 'v1' diff --git a/singlestoredb/fusion/README.md b/singlestoredb/fusion/README.md index 530e1b9e6..29d835575 100644 --- a/singlestoredb/fusion/README.md +++ b/singlestoredb/fusion/README.md @@ -208,7 +208,7 @@ ShowMonthHandler.register() Here is a more complete example demonstrating optional values, selection groups, and repeated values. It is abridged from `handlers/workspace.py`, which speaks -the deprecated management API v1 vocabulary; see `handlers/cluster.py` for the +the management API v1 vocabulary; see `handlers/cluster.py` for the current `CLUSTER` commands. ```python diff --git a/singlestoredb/fusion/handler.py b/singlestoredb/fusion/handler.py index 18b2f5458..64bed951e 100644 --- a/singlestoredb/fusion/handler.py +++ b/singlestoredb/fusion/handler.py @@ -585,10 +585,11 @@ class SQLHandler(NodeVisitor): _enabled: bool = True _preview: bool = False - #: Command that replaces this one, e.g. ``'SHOW CLUSTERS'``. When set, the - #: command still runs but warns on every execution. Used for the management - #: API v1 vocabulary (``handlers/workspace.py``), which v2 replaced with the - #: flat ``CLUSTER`` commands. Empty means not deprecated. + #: Command that replaces this one, e.g. ``'CREATE CLUSTER'``. When set, + #: the command still runs but warns on every execution. Used for the + #: management API v1 commands that create workspace groups and workspaces + #: (``handlers/workspace.py``), since new deployments should be clusters. + #: Empty means not deprecated. _deprecated_by: str = '' def __init__(self, connection: Connection): @@ -677,8 +678,8 @@ def execute(self, sql: str) -> result.FusionSQLResult: # After compile(), so that command_key is populated -- naming the # command the user actually typed is the point of the message. warnings.warn( - f'{" ".join(type(self).command_key).upper()} is a management ' - 'API v1 command and is deprecated. Use ' + f'{" ".join(type(self).command_key).upper()} is deprecated: ' + 'new deployments should be clusters. Use ' f'{type(self)._deprecated_by} instead.', DeprecatedFeatureWarning, stacklevel=2, ) diff --git a/singlestoredb/fusion/handlers/stage.py b/singlestoredb/fusion/handlers/stage.py index 87560685d..e670abb25 100644 --- a/singlestoredb/fusion/handlers/stage.py +++ b/singlestoredb/fusion/handlers/stage.py @@ -11,10 +11,9 @@ of Stage owner before v2 -- and which kind a given name belongs to is a fact about the org rather than about the statement. -``IN GROUP`` names a workspace group explicitly, and is the one deprecated -spelling here: it goes away with ``management/v1/``, and dropping the keyword -is an edit that works today either way. :func:`.utils.get_deployment` resolves -all of this, and everything it can return exposes ``.stage``. +``IN GROUP`` names a workspace group explicitly. +:func:`.utils.get_deployment` resolves all of this, and everything it can +return exposes ``.stage``. """ from typing import Any from typing import Dict @@ -94,9 +93,7 @@ class ShowStageFilesHandler(SQLHandler): * The ``IN`` clause specifies the ID or the name of the deployment -- or, for a Stage that has not moved off one, the workspace group -- in which the Stage is attached. - * The ``IN GROUP`` clause names a workspace group explicitly. It is - deprecated and goes away with management API v1, which is the version - workspace groups belong to: drop the ``GROUP`` keyword, since a bare + * The ``IN GROUP`` clause names a workspace group explicitly. A bare ``IN`` resolves a workspace group too. * Use the ``RECURSIVE`` clause to list the files recursively. * To return more information about the files, use the ``EXTENDED`` @@ -221,9 +218,7 @@ class UploadStageFileHandler(SQLHandler): * The ``IN`` clause specifies the ID or the name of the deployment -- or, for a Stage that has not moved off one, the workspace group -- in which the Stage is attached. - * The ``IN GROUP`` clause names a workspace group explicitly. It is - deprecated and goes away with management API v1, which is the version - workspace groups belong to: drop the ``GROUP`` keyword, since a bare + * The ``IN GROUP`` clause names a workspace group explicitly. A bare ``IN`` resolves a workspace group too. * If the ``OVERWRITE`` clause is specified, any existing file at the specified path in the Stage is overwritten. @@ -322,9 +317,7 @@ class DownloadStageFileHandler(SQLHandler): * The ``IN`` clause specifies the ID or the name of the deployment -- or, for a Stage that has not moved off one, the workspace group -- in which the Stage is attached. - * The ``IN GROUP`` clause names a workspace group explicitly. It is - deprecated and goes away with management API v1, which is the version - workspace groups belong to: drop the ``GROUP`` keyword, since a bare + * The ``IN GROUP`` clause names a workspace group explicitly. A bare ``IN`` resolves a workspace group too. * By default, files are downloaded in binary encoding. To view the contents of the file on the standard output, use the @@ -425,9 +418,7 @@ class DropStageFileHandler(SQLHandler): * The ``IN`` clause specifies the ID or the name of the deployment -- or, for a Stage that has not moved off one, the workspace group -- in which the Stage is attached. - * The ``IN GROUP`` clause names a workspace group explicitly. It is - deprecated and goes away with management API v1, which is the version - workspace groups belong to: drop the ``GROUP`` keyword, since a bare + * The ``IN GROUP`` clause names a workspace group explicitly. A bare ``IN`` resolves a workspace group too. Example @@ -507,9 +498,7 @@ class DropStageFolderHandler(SQLHandler): * The ``IN`` clause specifies the ID or the name of the deployment -- or, for a Stage that has not moved off one, the workspace group -- in which the Stage is attached. - * The ``IN GROUP`` clause names a workspace group explicitly. It is - deprecated and goes away with management API v1, which is the version - workspace groups belong to: drop the ``GROUP`` keyword, since a bare + * The ``IN GROUP`` clause names a workspace group explicitly. A bare ``IN`` resolves a workspace group too. Example @@ -590,9 +579,7 @@ class CreateStageFolderHandler(SQLHandler): * The ``IN`` clause specifies the ID or the name of the deployment -- or, for a Stage that has not moved off one, the workspace group -- in which the Stage is attached. - * The ``IN GROUP`` clause names a workspace group explicitly. It is - deprecated and goes away with management API v1, which is the version - workspace groups belong to: drop the ``GROUP`` keyword, since a bare + * The ``IN GROUP`` clause names a workspace group explicitly. A bare ``IN`` resolves a workspace group too. Example diff --git a/singlestoredb/fusion/handlers/utils.py b/singlestoredb/fusion/handlers/utils.py index c1cc9e2cd..285d43061 100644 --- a/singlestoredb/fusion/handlers/utils.py +++ b/singlestoredb/fusion/handlers/utils.py @@ -1,7 +1,6 @@ #!/usr/bin/env python import datetime import os -import warnings from typing import Any from typing import Dict from typing import Optional @@ -27,7 +26,6 @@ from ...management.workspace import Workspace from ...management.workspace import WorkspaceGroup from ...management.workspace import WorkspaceManager -from ...warnings import DeprecatedFeatureWarning def get_workspace_manager() -> WorkspaceManager: @@ -513,22 +511,6 @@ def _get_stage_group( if not group_name and not group_id: return None - # Warned before the lookup, so a caller who named a group that is gone - # still hears that the spelling itself is going. stacklevel reaches the - # handler method: user code is an unknown number of execute() frames - # further up, so there is no frame count that lands on it. - # - # The warning is about the clause, not the resource: a bare IN resolves a - # workspace group too, so dropping the GROUP keyword is an edit the caller - # can make today whether or not their Stage has moved to a cluster. - warnings.warn( - 'IN GROUP is deprecated: it names a workspace group explicitly, and ' - 'workspace groups are a management API v1 resource that goes away ' - 'with v1. Use a bare IN instead, which names a deployment or a ' - 'workspace group.', - DeprecatedFeatureWarning, stacklevel=3, - ) - group = _workspace_group(name=group_name, id=group_id) if group is None: raise KeyError( @@ -556,9 +538,7 @@ def _group_fallback( a cluster or a group is a fact about their org, not about their SQL. The group resource does go away with ``management/v1/``, but a warning here would ask for a migration that no edit to the statement can perform -- - the same reason :func:`.workspace._manage_workspaces_v1` exists. ``IN - GROUP`` still warns, because that spelling *is* something the user can - change. + the same reason :func:`.workspace._manage_workspaces_v1` exists. The deployment lookup goes first, so a name that is both a cluster's and a group's is the cluster's, and nothing that resolves today changes meaning. diff --git a/singlestoredb/fusion/handlers/workspace.py b/singlestoredb/fusion/handlers/workspace.py index 9bb44ce44..4ba05c46f 100644 --- a/singlestoredb/fusion/handlers/workspace.py +++ b/singlestoredb/fusion/handlers/workspace.py @@ -2,12 +2,12 @@ """ Fusion SQL handlers for the management API v1 workspace vocabulary. -**Deprecated.** ``handlers/cluster.py`` is the v2 replacement, and v2 is the -default everywhere else in the SDK. Every command here sets ``_deprecated_by`` -naming its ``CLUSTER`` counterpart, so it still runs but warns once per -execution. Nothing is removed and no grammar changed -- an existing v1 script -keeps working, it just says where to go. This module is what gets deleted when -``management/v1/`` goes. +``handlers/cluster.py`` holds the v2 ``CLUSTER`` commands, and v2 is the +default everywhere else in the SDK. Only the two commands that create +something -- ``CREATE WORKSPACE GROUP`` and ``CREATE WORKSPACE`` -- set +``_deprecated_by``, because new deployments should be clusters. The rest +manage workspace groups that already exist and run without warning. This +module is what gets deleted when ``management/v1/`` goes. Pinned to v1 through :func:`.utils.get_workspace_manager`: these commands *are* the v1 vocabulary, so they must not follow the ``management.version`` option onto @@ -91,7 +91,6 @@ class UseWorkspaceHandler(SQLHandler): USE WORKSPACE 'examplews' IN GROUP 'my-workspace-group'; """ - _deprecated_by = 'USE CLUSTER' def run(self, params: Dict[str, Any]) -> Optional[FusionSQLResult]: from singlestoredb.notebook import portal @@ -201,14 +200,6 @@ class ShowRegionsHandler(SQLHandler): """ - # Not a column-for-column replacement, unlike the rest of this module: v2 - # has no region IDs, so ``SHOW CLUSTER REGIONS`` reports ``Provider`` and - # ``RegionName`` where this reports ``ID``. Deprecated anyway, because this - # command reads the v1 API and that is what is going away -- a caller - # holding a v1 region ID needs to hear that now, not when the route stops - # answering. - _deprecated_by = 'SHOW CLUSTER REGIONS' - def run(self, params: Dict[str, Any]) -> Optional[FusionSQLResult]: manager = get_workspace_manager() @@ -267,8 +258,6 @@ class ShowWorkspaceGroupsHandler(SQLHandler): """ - _deprecated_by = 'SHOW CLUSTERS' - def run(self, params: Dict[str, Any]) -> Optional[FusionSQLResult]: manager = get_workspace_manager() @@ -360,8 +349,6 @@ class ShowWorkspacesHandler(SQLHandler): """ - _deprecated_by = 'SHOW CLUSTERS' - def run(self, params: Dict[str, Any]) -> Optional[FusionSQLResult]: res = FusionSQLResult() res.add_field('Name', result.STRING) @@ -747,8 +734,6 @@ class SuspendWorkspaceHandler(SQLHandler): """ # noqa: E501 - _deprecated_by = 'SUSPEND CLUSTER' - def run(self, params: Dict[str, Any]) -> Optional[FusionSQLResult]: ws = get_workspace(params) ws.suspend(wait_on_suspended=params['wait_on_suspended']) @@ -824,8 +809,6 @@ class ResumeWorkspaceHandler(SQLHandler): """ # noqa: E501 - _deprecated_by = 'RESUME CLUSTER' - def run(self, params: Dict[str, Any]) -> Optional[FusionSQLResult]: ws = get_workspace(params) ws.resume( @@ -894,8 +877,6 @@ class DropWorkspaceGroupHandler(SQLHandler): """ - _deprecated_by = 'DROP CLUSTER' - def run(self, params: Dict[str, Any]) -> Optional[FusionSQLResult]: try: workspace_group = get_workspace_group(params) @@ -984,8 +965,6 @@ class DropWorkspaceHandler(SQLHandler): """ - _deprecated_by = 'DROP CLUSTER' - def run(self, params: Dict[str, Any]) -> Optional[FusionSQLResult]: try: ws = get_workspace(params) diff --git a/singlestoredb/management/_version_import.py b/singlestoredb/management/_version_import.py index 254560e05..f53b320b6 100644 --- a/singlestoredb/management/_version_import.py +++ b/singlestoredb/management/_version_import.py @@ -2,13 +2,11 @@ """Importer for version-specific management API modules.""" import importlib import re -import warnings from typing import Any from typing import Optional from .. import config from .._management_version import DEFAULT_MANAGEMENT_VERSION -from .._management_version import DEPRECATED_MANAGEMENT_VERSION from ..exceptions import ManagementError @@ -21,52 +19,6 @@ #: :data:`singlestoredb._management_version.DEFAULT_MANAGEMENT_VERSION`. DEFAULT_VERSION = DEFAULT_MANAGEMENT_VERSION -#: The version this SDK is winding down. Everything under -#: ``singlestoredb.management.v1`` goes away with it, so any *public* entry -#: point that resolves to it warns -- see :func:`_warn_if_deprecated_version`. -DEPRECATED_VERSION = DEPRECATED_MANAGEMENT_VERSION - - -def _warn_if_deprecated_version(version: str, stacklevel: int = 3) -> None: - """ - Warn if ``version`` names a management API version being wound down. - - Called from the public version-neutral entry points -- the ``manage_*`` - factories and the ``get_organization``/``get_secret``/``get_stage`` - helpers -- *after* the version has been resolved, so it fires whether v1 - was named by the caller or inherited from the ``management.version`` - option. - - Deliberately not called from :func:`_resolve_version` itself. Several - internal paths are v1-only by design and resolve v1 with no v2 route to - move to -- ``workspace._manage_workspaces_v1`` and the inference API - behind it -- so warning at the resolver would emit noise the caller can do - nothing about. :func:`manage_workspaces` is likewise excluded: it raises - its own, more specific warning naming ``manage_clusters``. - - Parameters - ---------- - version : str - The already-resolved version - stacklevel : int, optional - Passed through to :func:`warnings.warn`. The default of 3 is right for - a public entry point calling this directly: 1 is this function, 2 is - the entry point, 3 is the user. Add one per intervening frame. - - """ - if version != DEPRECATED_VERSION: - return - warnings.warn( - f'management API {DEPRECATED_VERSION} is deprecated and will be ' - 'removed; it has been replaced by ' - f'{DEFAULT_VERSION}. Stop passing version=' - f'"{DEPRECATED_VERSION}", and unset the management.version option ' - '(the SINGLESTOREDB_MANAGEMENT_VERSION environment variable) if it ' - f'names {DEPRECATED_VERSION}.', - DeprecationWarning, - stacklevel=stacklevel + 1, - ) - def _resolve_version( version: Optional[str] = None, @@ -121,11 +73,6 @@ def _versioned_attr(name: str, version: Optional[str] = None) -> Any: future version is free to put them somewhere else again. Each version package re-exports its own, so this layer only has to resolve the version. - Every caller is a public entry point one frame up - (``organization.get_organization``, ``organization.get_secret``, - ``stage.get_stage``), so the deprecated-version warning is raised here - rather than repeated in each of them. - Parameters ---------- name : str @@ -145,8 +92,6 @@ def _versioned_attr(name: str, version: Optional[str] = None) -> Any: """ ver = _resolve_version(version) - # +1 for this frame sitting between the helper and the user. - _warn_if_deprecated_version(ver, stacklevel=4) pkg = _import_versioned_package(ver) try: return getattr(pkg, name) diff --git a/singlestoredb/management/files.py b/singlestoredb/management/files.py index 5a1b26f46..eacf6c7bb 100644 --- a/singlestoredb/management/files.py +++ b/singlestoredb/management/files.py @@ -647,8 +647,7 @@ def manage_files( version : str, optional Version of the API to use. Defaults to the ``management.version`` option (the ``SINGLESTOREDB_MANAGEMENT_VERSION`` environment - variable). ``'v1'`` is deprecated and raises a - :class:`DeprecationWarning`. + variable). base_url : str, optional Base URL of the files management API organization_id : str, optional @@ -661,9 +660,7 @@ def manage_files( """ from ._version_import import _import_versioned_module from ._version_import import _resolve_version - from ._version_import import _warn_if_deprecated_version ver = _resolve_version(version) - _warn_if_deprecated_version(ver) mod = _import_versioned_module(ver, 'files') return mod.FilesManager( access_token=access_token, base_url=base_url, diff --git a/singlestoredb/management/region.py b/singlestoredb/management/region.py index f5737ea06..d26b34ddf 100644 --- a/singlestoredb/management/region.py +++ b/singlestoredb/management/region.py @@ -156,8 +156,7 @@ def manage_regions( version : str, optional Version of the API to use. Defaults to the ``management.version`` option (the ``SINGLESTOREDB_MANAGEMENT_VERSION`` environment - variable). ``'v1'`` is deprecated and raises a - :class:`DeprecationWarning`. + variable). base_url : str, optional Base URL of the management API @@ -168,9 +167,7 @@ def manage_regions( """ from ._version_import import _import_versioned_module from ._version_import import _resolve_version - from ._version_import import _warn_if_deprecated_version ver = _resolve_version(version) - _warn_if_deprecated_version(ver) mod = _import_versioned_module(ver, 'region') return mod.RegionManager( access_token=access_token, diff --git a/singlestoredb/tests/test_fusion.py b/singlestoredb/tests/test_fusion.py index f4337e513..2013b0b25 100644 --- a/singlestoredb/tests/test_fusion.py +++ b/singlestoredb/tests/test_fusion.py @@ -261,19 +261,18 @@ def test_create_workspace_group_grammar_still_has_region_id(self): assert '' in syntax, syntax assert 'KMS' in syntax.upper(), syntax - def test_v1_workspace_commands_are_deprecated(self): + def test_only_v1_create_commands_are_deprecated(self): """ - Every v1 WORKSPACE command points at its v2 CLUSTER replacement. + Only the v1 commands that create a deployment warn. - No exceptions: every command in the module reads the v1 API, so every - one of them warns. ``SHOW REGIONS`` is the loosest pairing -- v2 assigns - no region IDs, so ``SHOW CLUSTER REGIONS`` reports ``RegionName`` where - it reports ``ID`` -- but it is still where a caller has to go. Asserted - so that adding a v1 command without a pointer fails here. + New deployments should be clusters, so ``CREATE WORKSPACE GROUP`` and + ``CREATE WORKSPACE`` point at ``CREATE CLUSTER``. The rest manage + workspace groups that already exist, and a warning there would tell + their owners to move to clusters when nothing requires it. """ from singlestoredb.fusion import registry - undeprecated = set() + deprecated = {} for key, handler in registry._handlers.items(): if not handler.__module__.endswith('.workspace'): continue @@ -281,10 +280,12 @@ def test_v1_workspace_commands_are_deprecated(self): # The replacement must be a real command, not a typo. assert handler._deprecated_by in registry._handlers, \ (key, handler._deprecated_by) - else: - undeprecated.add(key) + deprecated[key] = handler._deprecated_by - assert not undeprecated, undeprecated + assert deprecated == { + 'CREATE WORKSPACE GROUP': 'CREATE CLUSTER', + 'CREATE WORKSPACE': 'CREATE CLUSTER', + }, deprecated def test_v2_cluster_commands_are_not_deprecated(self): """The replacements must not themselves warn.""" @@ -742,18 +743,18 @@ def test_deployment_refuses_the_group_environment_variable(self): def test_in_group_resolves_a_workspace_group_against_v1(self): """ - ``IN GROUP`` names a v1 workspace group, by name and by ID. + ``IN GROUP`` names a v1 workspace group, by name and by ID, silently. Stage is attached to the group itself at v1, so a group names a Stage on its own. The cluster manager must not be touched at all: a group ID is not a cluster ID, and looking one up as the other is what made this spelling miss. """ + import warnings from unittest.mock import MagicMock from unittest.mock import patch from singlestoredb.fusion.handlers import utils - from singlestoredb.warnings import DeprecatedFeatureWarning group_id = '11111111-1111-4111-8111-111111111111' group = MagicMock() @@ -771,8 +772,11 @@ def resolve(params): utils, 'get_cluster_manager', return_value=clusters, ): self._fusion_env() - with self.assertWarns(DeprecatedFeatureWarning): - return utils.get_deployment(params) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + found = utils.get_deployment(params) + assert not caught, (params, [str(x.message) for x in caught]) + return found for params in ( dict(group=dict(group_name='wsg1')), @@ -792,11 +796,11 @@ def test_in_group_falls_back_to_a_starter_workspace(self): A starter workspace owns its Stage the same way a group does and was reachable through this spelling before, so it stays reachable. """ + import warnings from unittest.mock import MagicMock from unittest.mock import patch from singlestoredb.fusion.handlers import utils - from singlestoredb.warnings import DeprecatedFeatureWarning starter_id = '22222222-2222-4222-8222-222222222222' starter = MagicMock() @@ -812,8 +816,11 @@ def test_in_group_falls_back_to_a_starter_workspace(self): def resolve(params): with patch.object(utils, 'get_workspace_manager', return_value=v1): self._fusion_env() - with self.assertWarns(DeprecatedFeatureWarning): - return utils.get_deployment(params) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + found = utils.get_deployment(params) + assert not caught, (params, [str(x.message) for x in caught]) + return found for params in ( {'in': dict(in_group=dict(group_name='starter1'))}, @@ -827,9 +834,9 @@ def test_bare_in_falls_back_to_a_workspace_group(self): A bare ``IN`` named a workspace group before the Stage commands moved to v2 -- a group was the only kind of Stage owner there was -- so a - statement written then keeps working, and silently: ``IN`` is the - spelling to use for either kind of owner, so there is nothing about the - statement to warn about. Only ``IN GROUP`` warns. + statement written then keeps working, and silently: ``IN`` names + either kind of owner, so there is nothing about the statement to warn + about. """ import warnings from unittest.mock import MagicMock @@ -1035,16 +1042,13 @@ def test_stage_in_group_addresses_the_workspace_group(self): is empty, which is enough to prove the route was reached -- a cluster lookup would have raised instead. """ - from singlestoredb.warnings import DeprecatedFeatureWarning - wg = type(self).workspace_groups[0] for clause in [ f"in group id '{wg.id}'", f"in group '{wg.name}'", ]: - with self.assertWarns(DeprecatedFeatureWarning): - self.cur.execute(f'show stage files {clause}') + self.cur.execute(f'show stage files {clause}') assert len(list(self.cur)) == 0, clause def test_show_regions(self): diff --git a/singlestoredb/tests/test_management_versioning.py b/singlestoredb/tests/test_management_versioning.py index 97d2f2c19..164c7c67e 100644 --- a/singlestoredb/tests/test_management_versioning.py +++ b/singlestoredb/tests/test_management_versioning.py @@ -428,21 +428,21 @@ def test_internal_path_is_silent(self, _mock_token): ) -class TestDeprecatedVersionWarning(unittest.TestCase): +class TestV1VersionIsSilent(unittest.TestCase): """ - Every public version-neutral entry point warns when it resolves to v1. + The version-neutral entry points do not warn when they resolve to v1. - v1 is being wound down, so a caller who lands on it -- whether by passing - ``version='v1'`` or by inheriting it from the ``management.version`` - option -- has to be told. The warning fires after resolution rather than in - ``_resolve_version``, so both routes are covered and the internal v1-only - paths stay silent (see :class:`TestManageWorkspacesDeprecation`). + Owners of existing workspace groups still reach their resources through + v1, whether by passing ``version='v1'`` or by inheriting it from the + ``management.version`` option. A warning here would tell them to move to + clusters when nothing requires it; only ``manage_workspaces()`` and the + Fusion commands that create a workspace group or workspace warn. """ # (label, callable taking a version kwarg). Each is a public entry point # that can resolve to v1; ``manage_clusters`` is absent because v1 has no # clusters and it raises instead, and ``manage_workspaces`` because it - # raises its own more specific warning, asserted separately below. + # warns, asserted in :class:`TestManageWorkspacesDeprecation`. def _entry_points(self): import singlestoredb as s2 from singlestoredb.management import get_organization @@ -460,7 +460,7 @@ def _entry_points(self): ), ), # The three helpers dispatch through _versioned_attr, so they are - # patched out: the assertion is about the warning, not the route. + # patched out: the assertion is about warnings, not the route. ('get_organization', lambda **kw: get_organization(**kw)), ('get_secret', lambda **kw: get_secret('s', **kw)), ('get_stage', lambda **kw: get_stage('d', **kw)), @@ -482,29 +482,26 @@ def _stubbed_helpers(self): yield @patch('singlestoredb.management.manager.get_token', return_value=FAKE_TOKEN) - def test_explicit_v1_warns(self, _mock_token): + def test_explicit_v1_is_silent(self, _mock_token): with self._stubbed_helpers(): for label, call in self._entry_points(): with self.subTest(entry_point=label): - with self.assertWarns(DeprecationWarning) as ctx: + with warnings.catch_warnings(): + warnings.simplefilter('error', DeprecationWarning) call(version='v1') - msg = str(ctx.warning) - self.assertIn('v1', msg) - self.assertIn('deprecated', msg) @patch('singlestoredb.management.manager.get_token', return_value=FAKE_TOKEN) - def test_v1_inherited_from_the_option_warns(self, _mock_token): - """A caller who never names a version still gets told.""" + def test_v1_inherited_from_the_option_is_silent(self, _mock_token): with self._stubbed_helpers(), management_version('v1'): for label, call in self._entry_points(): with self.subTest(entry_point=label): - with self.assertWarns(DeprecationWarning) as ctx: + with warnings.catch_warnings(): + warnings.simplefilter('error', DeprecationWarning) call() - self.assertIn('deprecated', str(ctx.warning)) @patch('singlestoredb.management.manager.get_token', return_value=FAKE_TOKEN) def test_v2_is_silent(self, _mock_token): - """The default version must not warn -- otherwise nobody reads any of them.""" + """The default version does not warn either.""" with self._stubbed_helpers(), management_version('v2'): for label, call in self._entry_points() + [ ( @@ -523,15 +520,15 @@ def test_v2_is_silent(self, _mock_token): @patch('singlestoredb.management.manager.get_token', return_value=FAKE_TOKEN) def test_v1_still_works(self, _mock_token): """ - Deprecated must not mean broken. This is the point of the whole set. + v2 is the default, but v1 is still a supported version. - v2 is the default, but v1 is still a supported version: every entry - point must return a working v1 object, and none may raise merely - because the default moved. Warnings are the only consequence. + Every entry point must return a working v1 object, and none may raise + merely because the default moved. """ import singlestoredb as s2 from singlestoredb.management.workspace import manage_workspaces with self._stubbed_helpers(), warnings.catch_warnings(): + # manage_workspaces() still warns; that is asserted elsewhere. warnings.simplefilter('ignore', DeprecationWarning) for label, call in self._entry_points(): with self.subTest(entry_point=label): @@ -550,25 +547,19 @@ def test_v1_still_works(self, _mock_token): ) self.assertIn('/v1/', mgr._base_url) - def test_the_deprecated_version_is_not_the_default(self): - """Guards the pair: whatever DEPRECATED_VERSION names cannot be the default.""" + def test_the_default_version_is_v2(self): from singlestoredb import config from singlestoredb.management import _version_import as vi - self.assertNotEqual(vi.DEPRECATED_VERSION, vi.DEFAULT_VERSION) self.assertEqual(vi.DEFAULT_VERSION, 'v2') - self.assertNotEqual( - config.get_default('management.version'), vi.DEPRECATED_VERSION, - ) + self.assertEqual(config.get_default('management.version'), 'v2') @patch('singlestoredb.management.manager.get_token', return_value=FAKE_TOKEN) def test_manage_workspaces_warns_once_not_twice(self, _mock_token): """ - ``manage_workspaces()`` is the one v1 entry point with its own message. + ``manage_workspaces()`` warns exactly once. It reaches v1 through ``_manage_workspaces_v1``, which is deliberately - silent, so the caller gets exactly one warning -- the specific one - naming ``manage_clusters`` -- rather than that plus the generic - "v1 is deprecated". + silent, so the caller gets only the warning naming ``manage_clusters``. """ from singlestoredb.management.workspace import manage_workspaces with warnings.catch_warnings(record=True) as caught: