From 237c0d3d68a2f5b844e0ee02ab1f3324cde722c6 Mon Sep 17 00:00:00 2001 From: Sri Harsha CH Date: Sat, 18 Nov 2023 08:39:15 +0000 Subject: [PATCH 1/8] feat(spanner): add code changes for managed autoscalar --- google/cloud/spanner_v1/client.py | 2 + google/cloud/spanner_v1/instance.py | 24 ++++++++- samples/samples/snippets.py | 46 ++++++++++++++++ samples/samples/snippets_test.py | 12 +++++ tests/system/test_instance_api.py | 46 ++++++++++++++++ tests/unit/test_client.py | 24 ++++++++- tests/unit/test_instance.py | 84 +++++++++++++++++++++++++++++ 7 files changed, 235 insertions(+), 3 deletions(-) diff --git a/google/cloud/spanner_v1/client.py b/google/cloud/spanner_v1/client.py index a0e848228b..1eb1681d57 100644 --- a/google/cloud/spanner_v1/client.py +++ b/google/cloud/spanner_v1/client.py @@ -309,6 +309,7 @@ def instance( node_count=None, labels=None, processing_units=None, + autoscaling_config=None, ): """Factory to create a instance associated with this client. @@ -352,6 +353,7 @@ def instance( self._emulator_host, labels, processing_units, + autoscaling_config, ) def list_instances(self, filter_="", page_size=None): diff --git a/google/cloud/spanner_v1/instance.py b/google/cloud/spanner_v1/instance.py index 1b426f8cc2..06bcd9e2f4 100644 --- a/google/cloud/spanner_v1/instance.py +++ b/google/cloud/spanner_v1/instance.py @@ -122,6 +122,7 @@ def __init__( emulator_host=None, labels=None, processing_units=None, + autoscaling_config=None, ): self.instance_id = instance_id self._client = client @@ -131,20 +132,28 @@ def __init__( raise InvalidArgument( "Only one of node count and processing units can be set." ) - if node_count is None and processing_units is None: + if ( + node_count is None + and processing_units is None + and autoscaling_config is None + ): self._node_count = DEFAULT_NODE_COUNT self._processing_units = DEFAULT_NODE_COUNT * PROCESSING_UNITS_PER_NODE elif node_count is not None: self._node_count = node_count self._processing_units = node_count * PROCESSING_UNITS_PER_NODE - else: + elif processing_units is not None: self._processing_units = processing_units self._node_count = processing_units // PROCESSING_UNITS_PER_NODE + else: + self._node_count = None + self._processing_units = None self.display_name = display_name or instance_id self.emulator_host = emulator_host if labels is None: labels = {} self.labels = labels + self._autoscaling_config = autoscaling_config def _update_from_pb(self, instance_pb): """Refresh self from the server-provided protobuf. @@ -158,6 +167,7 @@ def _update_from_pb(self, instance_pb): self._node_count = instance_pb.node_count self._processing_units = instance_pb.processing_units self.labels = instance_pb.labels + self._autoscaling_config = instance_pb.autoscaling_config @classmethod def from_pb(cls, instance_pb, client): @@ -250,6 +260,14 @@ def node_count(self, value): self._node_count = value self._processing_units = value * PROCESSING_UNITS_PER_NODE + @property + def autoscaling_config(self): + return self._autoscaling_config + + @autoscaling_config.setter + def autoscaling_config(self, value): + self._autoscaling_config = value + def __eq__(self, other): if not isinstance(other, self.__class__): return NotImplemented @@ -281,6 +299,7 @@ def copy(self): node_count=self._node_count, processing_units=self._processing_units, display_name=self.display_name, + autoscaling_config=self._autoscaling_config, ) def create(self): @@ -313,6 +332,7 @@ def create(self): display_name=self.display_name, processing_units=self._processing_units, labels=self.labels, + autoscaling_config=self._autoscaling_config, ) metadata = _metadata_with_prefix(self.name) diff --git a/samples/samples/snippets.py b/samples/samples/snippets.py index 82fb95a0dd..91f1672790 100644 --- a/samples/samples/snippets.py +++ b/samples/samples/snippets.py @@ -107,6 +107,52 @@ def create_instance_with_processing_units(instance_id, processing_units): # [END spanner_create_instance_with_processing_units] +# [START spanner_create_instance_with_autoscaling_config] +def create_instance_with_autoscaling_config(instance_id): + """Creates an instance.""" + spanner_client = spanner.Client() + + config_name = "{}/instanceConfigs/regional-us-west1".format( + spanner_client.project_name + ) + + autoscaling_config = spanner_instance_admin.AutoscalingConfig( + autoscaling_limits=spanner_instance_admin.AutoscalingConfig.AutoscalingLimits( + min_nodes=1, + max_nodes=2, + ), + autoscaling_targets=spanner_instance_admin.AutoscalingConfig.AutoscalingTargets( + high_priority_cpu_utilization_percent=65, + storage_utilization_percent=95, + ), + ) + + instance = spanner_client.instance( + instance_id, + configuration_name=config_name, + display_name="Autoscaling config instance.", + labels={ + "cloud_spanner_samples": "true", + "sample_name": "snippets-create_instance_with_autoscaling_config", + "created": str(int(time.time())), + }, + autoscaling_config=autoscaling_config, + ) + + operation = instance.create() + + print("Waiting for operation to complete...") + operation.result(OPERATION_TIMEOUT_SECONDS) + print( + "Created instance {} with {} autoscaling config".format( + instance_id, instance.autoscaling_config + ) + ) + + +# [END spanner_create_instance_with_autoscaling_config] + + # [START spanner_get_instance_config] def get_instance_config(instance_config): """Gets the leader options for the instance configuration.""" diff --git a/samples/samples/snippets_test.py b/samples/samples/snippets_test.py index 22b5b6f944..f4d4c7ca95 100644 --- a/samples/samples/snippets_test.py +++ b/samples/samples/snippets_test.py @@ -154,6 +154,18 @@ def test_create_instance_with_processing_units(capsys, lci_instance_id): retry_429(instance.delete)() +def test_create_instance_with_autoscaling_config(capsys, lci_instance_id): + retry_429(snippets.create_instance_with_autoscaling_config)( + lci_instance_id, + ) + out, _ = capsys.readouterr() + assert lci_instance_id in out + assert "autoscaling config" in out + spanner_client = spanner.Client() + instance = spanner_client.instance(lci_instance_id) + retry_429(instance.delete)() + + def test_update_database(capsys, instance_id, sample_database): snippets.update_database(instance_id, sample_database.database_id) out, _ = capsys.readouterr() diff --git a/tests/system/test_instance_api.py b/tests/system/test_instance_api.py index 6825e50721..d1f0c3f1ae 100644 --- a/tests/system/test_instance_api.py +++ b/tests/system/test_instance_api.py @@ -140,3 +140,49 @@ def test_update_instance( # other test cases. shared_instance.display_name = old_display_name shared_instance.update() + + +def test_create_instance_with_autoscaling_config( + not_emulator, + if_create_instance, + spanner_client, + instance_config, + instances_to_delete, + instance_operation_timeout, +): + from google.cloud.spanner_admin_instance_v1 import ( + AutoscalingConfig as AutoscalingConfigPB, + ) + + autoscaling_config = AutoscalingConfigPB( + autoscaling_limits=AutoscalingConfigPB.AutoscalingLimits( + min_nodes=1, + max_nodes=2, + ), + autoscaling_targets=AutoscalingConfigPB.AutoscalingTargets( + high_priority_cpu_utilization_percent=65, + storage_utilization_percent=95, + ), + ) + + alt_instance_id = _helpers.unique_id("wpn") + instance = spanner_client.instance( + instance_id=alt_instance_id, + configuration_name=instance_config.name, + autoscaling_config=autoscaling_config, + ) + operation = instance.create() + # Make sure this instance gets deleted after the test case. + instances_to_delete.append(instance) + + # We want to make sure the operation completes. + operation.result(instance_operation_timeout) # raises on failure / timeout. + + # Create a new instance instance and make sure it is the same. + instance_alt = spanner_client.instance(alt_instance_id, instance_config.name) + instance_alt.reload() + + assert instance == instance_alt + assert instance.display_name == instance_alt.display_name + pdb.set_trace() + assert instance.autoscaling_config == instance_alt.autoscaling_config diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 049ee1124f..570a2fba11 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -587,11 +587,24 @@ def test_list_instances(self): from google.cloud.spanner_admin_instance_v1 import Instance as InstancePB from google.cloud.spanner_admin_instance_v1 import ListInstancesRequest from google.cloud.spanner_admin_instance_v1 import ListInstancesResponse + from google.cloud.spanner_admin_instance_v1 import ( + AutoscalingConfig as AutoscalingConfigPB, + ) api = InstanceAdminClient(credentials=mock.Mock()) credentials = _make_credentials() client = self._make_one(project=self.PROJECT, credentials=credentials) client._instance_admin_api = api + autoscaling_config = AutoscalingConfigPB( + autoscaling_limits=AutoscalingConfigPB.AutoscalingLimits( + min_nodes=1, + max_nodes=2, + ), + autoscaling_targets=AutoscalingConfigPB.AutoscalingTargets( + high_priority_cpu_utilization_percent=65, + storage_utilization_percent=95, + ), + ) instance_pbs = ListInstancesResponse( instances=[ @@ -601,7 +614,11 @@ def test_list_instances(self): display_name=self.DISPLAY_NAME, node_count=self.NODE_COUNT, processing_units=self.PROCESSING_UNITS, - ) + ), + InstancePB( + name=self.INSTANCE_NAME, + autoscaling_config=autoscaling_config, + ), ] ) @@ -620,6 +637,11 @@ def test_list_instances(self): self.assertEqual(instance.node_count, self.NODE_COUNT) self.assertEqual(instance.processing_units, self.PROCESSING_UNITS) + instance1 = instances[1] + self.assertIsInstance(instance1, InstancePB) + self.assertEqual(instance.name, self.INSTANCE_NAME) + self.assertEqual(instance1.autoscaling_config, autoscaling_config) + expected_metadata = ( ("google-cloud-resource-prefix", client.project_name), ("x-goog-request-params", "parent={}".format(client.project_name)), diff --git a/tests/unit/test_instance.py b/tests/unit/test_instance.py index 20064e7e88..daf07c957b 100644 --- a/tests/unit/test_instance.py +++ b/tests/unit/test_instance.py @@ -59,6 +59,7 @@ def test_constructor_defaults(self): self.assertEqual(instance.node_count, DEFAULT_NODE_COUNT) self.assertEqual(instance.display_name, self.INSTANCE_ID) self.assertEqual(instance.labels, {}) + self.assertEqual(instance.autoscaling_config, None) def test_constructor_non_default(self): DISPLAY_NAME = "display_name" @@ -291,6 +292,49 @@ def test_create_with_processing_units(self): self.assertEqual(instance.labels, self.LABELS) self.assertEqual(metadata, [("google-cloud-resource-prefix", instance.name)]) + def test_create_with_autoscaling_config(self): + from google.cloud.spanner_admin_instance_v1 import ( + AutoscalingConfig as AutoscalingConfigPB, + ) + + op_future = _FauxOperationFuture() + client = _Client(self.PROJECT) + autoscaling_config = AutoscalingConfigPB( + autoscaling_limits=AutoscalingConfigPB.AutoscalingLimits( + min_nodes=1, + max_nodes=2, + ), + autoscaling_targets=AutoscalingConfigPB.AutoscalingTargets( + high_priority_cpu_utilization_percent=65, + storage_utilization_percent=95, + ), + ) + api = client.instance_admin_api = _FauxInstanceAdminAPI( + _create_instance_response=op_future + ) + instance = self._make_one( + self.INSTANCE_ID, + client, + configuration_name=self.CONFIG_NAME, + display_name=self.DISPLAY_NAME, + labels=self.LABELS, + autoscaling_config=autoscaling_config, + ) + + future = instance.create() + + self.assertIs(future, op_future) + + (parent, instance_id, instance, metadata) = api._created_instance + self.assertEqual(parent, self.PARENT) + self.assertEqual(instance_id, self.INSTANCE_ID) + self.assertEqual(instance.name, self.INSTANCE_NAME) + self.assertEqual(instance.config, self.CONFIG_NAME) + self.assertEqual(instance.display_name, self.DISPLAY_NAME) + self.assertEqual(instance.labels, self.LABELS) + self.assertEqual(metadata, [("google-cloud-resource-prefix", instance.name)]) + self.assertEqual(instance.autoscaling_config, autoscaling_config) + def test_exists_instance_grpc_error(self): from google.api_core.exceptions import Unknown @@ -390,6 +434,46 @@ def test_reload_success(self): self.assertEqual(name, self.INSTANCE_NAME) self.assertEqual(metadata, [("google-cloud-resource-prefix", instance.name)]) + def test_reload_with_autoscaling_config(self): + from google.cloud.spanner_admin_instance_v1 import Instance + from google.cloud.spanner_admin_instance_v1 import ( + AutoscalingConfig as AutoscalingConfigPB, + ) + + autoscaling_config = AutoscalingConfigPB( + autoscaling_limits=AutoscalingConfigPB.AutoscalingLimits( + min_nodes=1, + max_nodes=2, + ), + autoscaling_targets=AutoscalingConfigPB.AutoscalingTargets( + high_priority_cpu_utilization_percent=65, + storage_utilization_percent=95, + ), + ) + client = _Client(self.PROJECT) + instance_pb = Instance( + name=self.INSTANCE_NAME, + config=self.CONFIG_NAME, + display_name=self.DISPLAY_NAME, + labels=self.LABELS, + autoscaling_config=autoscaling_config, + ) + api = client.instance_admin_api = _FauxInstanceAdminAPI( + _get_instance_response=instance_pb + ) + instance = self._make_one(self.INSTANCE_ID, client) + + instance.reload() + + self.assertEqual(instance.configuration_name, self.CONFIG_NAME) + self.assertEqual(instance.display_name, self.DISPLAY_NAME) + self.assertEqual(instance.labels, self.LABELS) + self.assertEqual(instance.autoscaling_config, autoscaling_config) + + name, metadata = api._got_instance + self.assertEqual(name, self.INSTANCE_NAME) + self.assertEqual(metadata, [("google-cloud-resource-prefix", instance.name)]) + def test_update_grpc_error(self): from google.api_core.exceptions import Unknown From b2c22db48b4a1f355ae6aa8b7842e257e58ad60d Mon Sep 17 00:00:00 2001 From: Sri Harsha CH Date: Sat, 18 Nov 2023 09:31:26 +0000 Subject: [PATCH 2/8] feat(spanner): remove pdb --- tests/system/test_instance_api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/system/test_instance_api.py b/tests/system/test_instance_api.py index d1f0c3f1ae..e1623cacee 100644 --- a/tests/system/test_instance_api.py +++ b/tests/system/test_instance_api.py @@ -184,5 +184,4 @@ def test_create_instance_with_autoscaling_config( assert instance == instance_alt assert instance.display_name == instance_alt.display_name - pdb.set_trace() assert instance.autoscaling_config == instance_alt.autoscaling_config From d67f5f30ed86af6a39f788524fe93f64556df85a Mon Sep 17 00:00:00 2001 From: Sri Harsha CH Date: Sat, 18 Nov 2023 12:47:27 +0000 Subject: [PATCH 3/8] feat(spanner): update support for autoscaling config field --- google/cloud/spanner_v1/client.py | 4 +++ google/cloud/spanner_v1/instance.py | 21 ++++++++--- tests/system/test_instance_api.py | 47 +++++++++++++++++++++++++ tests/unit/test_instance.py | 54 +++++++++++++++++++++++++++-- 4 files changed, 120 insertions(+), 6 deletions(-) diff --git a/google/cloud/spanner_v1/client.py b/google/cloud/spanner_v1/client.py index 1eb1681d57..972fb93ac9 100644 --- a/google/cloud/spanner_v1/client.py +++ b/google/cloud/spanner_v1/client.py @@ -341,6 +341,10 @@ def instance( :type labels: dict (str -> str) or None :param labels: (Optional) User-assigned labels for this instance. + :type autoscaling_config: + :class:`~google.cloud.spanner_admin_instance_v1.types.AutoscalingConfig` + :param autoscaling_config: (Optional) The autoscaling configuration for this instance. + :rtype: :class:`~google.cloud.spanner_v1.instance.Instance` :returns: an instance owned by this client. """ diff --git a/google/cloud/spanner_v1/instance.py b/google/cloud/spanner_v1/instance.py index 06bcd9e2f4..fcb89108dd 100644 --- a/google/cloud/spanner_v1/instance.py +++ b/google/cloud/spanner_v1/instance.py @@ -379,7 +379,7 @@ def reload(self): self._update_from_pb(instance_pb) - def update(self): + def update(self, fields=None): """Update this instance. See @@ -397,6 +397,10 @@ def update(self): before calling :meth:`update`. + :type fields: Sequence[str] + :param fields: a list of fields to update. Ex: ["config", "display_name", + "processing_units", "labels","autoscaling_config"] + :rtype: :class:`google.api_core.operation.Operation` :returns: an operation instance :raises NotFound: if the instance does not exist @@ -409,12 +413,21 @@ def update(self): node_count=self._node_count, processing_units=self._processing_units, labels=self.labels, + autoscaling_config=self._autoscaling_config, ) + # default field paths to update + paths = [ + "config", + "display_name", + "processing_units", + "labels", + "autoscaling_config", + ] + if fields is not None: + paths = fields # Always update only processing_units, not nodes - field_mask = FieldMask( - paths=["config", "display_name", "processing_units", "labels"] - ) + field_mask = FieldMask(paths=paths) metadata = _metadata_with_prefix(self.name) future = api.update_instance( diff --git a/tests/system/test_instance_api.py b/tests/system/test_instance_api.py index e1623cacee..bcbb7db77e 100644 --- a/tests/system/test_instance_api.py +++ b/tests/system/test_instance_api.py @@ -185,3 +185,50 @@ def test_create_instance_with_autoscaling_config( assert instance == instance_alt assert instance.display_name == instance_alt.display_name assert instance.autoscaling_config == instance_alt.autoscaling_config + + +def test_update_instance_with_autoscaling_config( + not_emulator, + spanner_client, + shared_instance, + shared_instance_id, + instance_operation_timeout, +): + from google.cloud.spanner_admin_instance_v1 import ( + AutoscalingConfig as AutoscalingConfigPB, + ) + + autoscaling_config = AutoscalingConfigPB( + autoscaling_limits=AutoscalingConfigPB.AutoscalingLimits( + min_nodes=1, + max_nodes=2, + ), + autoscaling_targets=AutoscalingConfigPB.AutoscalingTargets( + high_priority_cpu_utilization_percent=65, + storage_utilization_percent=95, + ), + ) + assert shared_instance.autoscaling_config != autoscaling_config + + old_node_count = shared_instance.node_count + shared_instance.autoscaling_config = autoscaling_config + # Update only the autoscaling_config field. This is to ensure that + # node_count or processing_unit field is not considered during update, + # which might otherwise throw an error. + operation = shared_instance.update(fields=["autoscaling_config"]) + + # We want to make sure the operation completes. + operation.result(instance_operation_timeout) # raises on failure / timeout. + + # Create a new instance instance and reload it. + instance_alt = spanner_client.instance(shared_instance_id, None) + assert instance_alt.autoscaling_config != autoscaling_config + + instance_alt.reload() + assert instance_alt.autoscaling_config == autoscaling_config + + # Make sure to put the instance back the way it was for the + # other test cases. + shared_instance.node_count = old_node_count + shared_instance.autoscaling_config = None + shared_instance.update() diff --git a/tests/unit/test_instance.py b/tests/unit/test_instance.py index daf07c957b..e1e419c3ba 100644 --- a/tests/unit/test_instance.py +++ b/tests/unit/test_instance.py @@ -38,7 +38,13 @@ class TestInstance(unittest.TestCase): DATABASE_ID = "database_id" DATABASE_NAME = "%s/databases/%s" % (INSTANCE_NAME, DATABASE_ID) LABELS = {"test": "true"} - FIELD_MASK = ["config", "display_name", "processing_units", "labels"] + FIELD_MASK = [ + "config", + "display_name", + "processing_units", + "labels", + "autoscaling_config", + ] def _getTargetClass(self): from google.cloud.spanner_v1.instance import Instance @@ -558,7 +564,14 @@ def test_update_success_with_processing_units(self): instance, field_mask, metadata = api._updated_instance self.assertEqual( - field_mask.paths, ["config", "display_name", "processing_units", "labels"] + field_mask.paths, + [ + "config", + "display_name", + "processing_units", + "labels", + "autoscaling_config", + ], ) self.assertEqual(instance.name, self.INSTANCE_NAME) self.assertEqual(instance.config, self.CONFIG_NAME) @@ -567,6 +580,43 @@ def test_update_success_with_processing_units(self): self.assertEqual(instance.labels, self.LABELS) self.assertEqual(metadata, [("google-cloud-resource-prefix", instance.name)]) + def test_update_success_with_autoscaling_config(self): + from google.cloud.spanner_admin_instance_v1 import ( + AutoscalingConfig as AutoscalingConfigPB, + ) + + autoscaling_config = AutoscalingConfigPB( + autoscaling_limits=AutoscalingConfigPB.AutoscalingLimits( + min_nodes=1, + max_nodes=2, + ), + autoscaling_targets=AutoscalingConfigPB.AutoscalingTargets( + high_priority_cpu_utilization_percent=65, + storage_utilization_percent=95, + ), + ) + + op_future = _FauxOperationFuture() + client = _Client(self.PROJECT) + api = client.instance_admin_api = _FauxInstanceAdminAPI( + _update_instance_response=op_future + ) + instance = self._make_one( + self.INSTANCE_ID, + client, + autoscaling_config=autoscaling_config, + ) + + future = instance.update(fields=["autoscaling_config"]) + + self.assertIs(future, op_future) + + instance, field_mask, metadata = api._updated_instance + self.assertEqual(field_mask.paths, ["autoscaling_config"]) + self.assertEqual(instance.name, self.INSTANCE_NAME) + self.assertEqual(instance.autoscaling_config, autoscaling_config) + self.assertEqual(metadata, [("google-cloud-resource-prefix", instance.name)]) + def test_delete_grpc_error(self): from google.api_core.exceptions import Unknown From 5dfdf10927fdfb207e1eb6df3750a9b62d47869d Mon Sep 17 00:00:00 2001 From: Sri Harsha CH Date: Sat, 18 Nov 2023 13:29:07 +0000 Subject: [PATCH 4/8] feat(spanner): fix lint --- google/cloud/spanner_v1/instance.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/google/cloud/spanner_v1/instance.py b/google/cloud/spanner_v1/instance.py index fcb89108dd..d3604a35a9 100644 --- a/google/cloud/spanner_v1/instance.py +++ b/google/cloud/spanner_v1/instance.py @@ -398,8 +398,7 @@ def update(self, fields=None): before calling :meth:`update`. :type fields: Sequence[str] - :param fields: a list of fields to update. Ex: ["config", "display_name", - "processing_units", "labels","autoscaling_config"] + :param fields: a list of fields to update. Ex: ["autoscaling_config"] :rtype: :class:`google.api_core.operation.Operation` :returns: an operation instance From 63e4d317cc0bc3c0715575c0a6e7fcb7f61f366c Mon Sep 17 00:00:00 2001 From: Sri Harsha CH Date: Fri, 9 Feb 2024 06:35:20 +0000 Subject: [PATCH 5/8] feat(spanner): comment refactoring --- google/cloud/spanner_v1/client.py | 10 ++++++++-- google/cloud/spanner_v1/instance.py | 6 ++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/google/cloud/spanner_v1/client.py b/google/cloud/spanner_v1/client.py index 0be6156465..85b9b71d80 100644 --- a/google/cloud/spanner_v1/client.py +++ b/google/cloud/spanner_v1/client.py @@ -351,11 +351,15 @@ def instance( :type node_count: int :param node_count: (Optional) The number of nodes in the instance's - cluster; used to set up the instance's cluster. + cluster; used to set up the instance's cluster. Exactly one of + processing units, node count, or autoscaling config must be set + when creating a new instance. :type processing_units: int :param processing_units: (Optional) The number of processing units - allocated to this instance. + allocated to this instance. Exactly one of + processing units, node count, or autoscaling config must be set + when creating a new instance. :type labels: dict (str -> str) or None :param labels: (Optional) User-assigned labels for this instance. @@ -363,6 +367,8 @@ def instance( :type autoscaling_config: :class:`~google.cloud.spanner_admin_instance_v1.types.AutoscalingConfig` :param autoscaling_config: (Optional) The autoscaling configuration for this instance. + Exactly one of processing units, node count, or autoscaling config + must be set when creating a new instance. :rtype: :class:`~google.cloud.spanner_v1.instance.Instance` :returns: an instance owned by this client. diff --git a/google/cloud/spanner_v1/instance.py b/google/cloud/spanner_v1/instance.py index c6c9893682..b927586ece 100644 --- a/google/cloud/spanner_v1/instance.py +++ b/google/cloud/spanner_v1/instance.py @@ -110,6 +110,12 @@ class Instance(object): :type labels: dict (str -> str) or None :param labels: (Optional) User-assigned labels for this instance. + + :type autoscaling_config: + :class:`~google.cloud.spanner_admin_instance_v1.types.AutoscalingConfig` + :param autoscaling_config: (Optional) The autoscaling configuration for this instance. + Exactly one of processing units, node count, or autoscaling config + must be set when creating a new instance. """ def __init__( From f53c3bc90163151a6c35c2b9e960bad7ca1ae7f4 Mon Sep 17 00:00:00 2001 From: Sri Harsha CH Date: Fri, 9 Feb 2024 06:54:18 +0000 Subject: [PATCH 6/8] feat(spanner): comment refactor --- google/cloud/spanner_v1/instance.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/spanner_v1/instance.py b/google/cloud/spanner_v1/instance.py index b927586ece..c3f00d0309 100644 --- a/google/cloud/spanner_v1/instance.py +++ b/google/cloud/spanner_v1/instance.py @@ -421,6 +421,7 @@ def update(self, fields=None): autoscaling_config=self._autoscaling_config, ) # default field paths to update + # Always update only processing_units, not nodes paths = [ "config", "display_name", @@ -431,7 +432,6 @@ def update(self, fields=None): if fields is not None: paths = fields - # Always update only processing_units, not nodes field_mask = FieldMask(paths=paths) metadata = _metadata_with_prefix(self.name) From b221c4d139d0bd5a68b3709503fadf030134bf14 Mon Sep 17 00:00:00 2001 From: Sri Harsha CH Date: Fri, 9 Feb 2024 07:07:03 +0000 Subject: [PATCH 7/8] feat(spanner): update instance method refactor --- google/cloud/spanner_v1/instance.py | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/google/cloud/spanner_v1/instance.py b/google/cloud/spanner_v1/instance.py index c3f00d0309..f69762acb6 100644 --- a/google/cloud/spanner_v1/instance.py +++ b/google/cloud/spanner_v1/instance.py @@ -385,7 +385,7 @@ def reload(self): self._update_from_pb(instance_pb) - def update(self, fields=None): + def update(self): """Update this instance. See @@ -403,9 +403,6 @@ def update(self, fields=None): before calling :meth:`update`. - :type fields: Sequence[str] - :param fields: a list of fields to update. Ex: ["autoscaling_config"] - :rtype: :class:`google.api_core.operation.Operation` :returns: an operation instance :raises NotFound: if the instance does not exist @@ -420,8 +417,11 @@ def update(self, fields=None): labels=self.labels, autoscaling_config=self._autoscaling_config, ) - # default field paths to update - # Always update only processing_units, not nodes + # default field paths to update. Always update only processing_units, + # not nodes. + # When switching from autoscalar to non-autoscalar + # instance, we need to update processing_units value and set + # autoscaling_config to None paths = [ "config", "display_name", @@ -429,8 +429,18 @@ def update(self, fields=None): "labels", "autoscaling_config", ] - if fields is not None: - paths = fields + if self._autoscaling_config is not None: + # When switching from non-autoscalar to autoscalar instance, + # update only autoscaling_config. An update cannot simultaneously + # set processing_units to non-zero and autoscaling_config to + # non-empty. + paths = [ + "config", + "display_name", + "labels", + "autoscaling_config", + ] + field_mask = FieldMask(paths=paths) metadata = _metadata_with_prefix(self.name) From 9373f02633af3caefbb4531811e127bf74cb9f21 Mon Sep 17 00:00:00 2001 From: Sri Harsha CH Date: Fri, 9 Feb 2024 07:33:09 +0000 Subject: [PATCH 8/8] feat(spanner): update tests --- google/cloud/spanner_v1/instance.py | 1 - tests/system/test_instance_api.py | 7 ++----- tests/unit/test_instance.py | 12 ++++++++++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/google/cloud/spanner_v1/instance.py b/google/cloud/spanner_v1/instance.py index f69762acb6..c66034c965 100644 --- a/google/cloud/spanner_v1/instance.py +++ b/google/cloud/spanner_v1/instance.py @@ -441,7 +441,6 @@ def update(self): "autoscaling_config", ] - field_mask = FieldMask(paths=paths) metadata = _metadata_with_prefix(self.name) diff --git a/tests/system/test_instance_api.py b/tests/system/test_instance_api.py index bcbb7db77e..9d3733236a 100644 --- a/tests/system/test_instance_api.py +++ b/tests/system/test_instance_api.py @@ -178,7 +178,7 @@ def test_create_instance_with_autoscaling_config( # We want to make sure the operation completes. operation.result(instance_operation_timeout) # raises on failure / timeout. - # Create a new instance instance and make sure it is the same. + # Create a new instance and make sure it is the same. instance_alt = spanner_client.instance(alt_instance_id, instance_config.name) instance_alt.reload() @@ -212,10 +212,7 @@ def test_update_instance_with_autoscaling_config( old_node_count = shared_instance.node_count shared_instance.autoscaling_config = autoscaling_config - # Update only the autoscaling_config field. This is to ensure that - # node_count or processing_unit field is not considered during update, - # which might otherwise throw an error. - operation = shared_instance.update(fields=["autoscaling_config"]) + operation = shared_instance.update() # We want to make sure the operation completes. operation.result(instance_operation_timeout) # raises on failure / timeout. diff --git a/tests/unit/test_instance.py b/tests/unit/test_instance.py index 627f93241d..b46f960c7f 100644 --- a/tests/unit/test_instance.py +++ b/tests/unit/test_instance.py @@ -607,12 +607,20 @@ def test_update_success_with_autoscaling_config(self): autoscaling_config=autoscaling_config, ) - future = instance.update(fields=["autoscaling_config"]) + future = instance.update() self.assertIs(future, op_future) instance, field_mask, metadata = api._updated_instance - self.assertEqual(field_mask.paths, ["autoscaling_config"]) + self.assertEqual( + field_mask.paths, + [ + "config", + "display_name", + "labels", + "autoscaling_config", + ], + ) self.assertEqual(instance.name, self.INSTANCE_NAME) self.assertEqual(instance.autoscaling_config, autoscaling_config) self.assertEqual(metadata, [("google-cloud-resource-prefix", instance.name)])