From 499799e594303cf38a35fd6c8d77d54a44506855 Mon Sep 17 00:00:00 2001 From: larkee Date: Tue, 15 Dec 2020 10:25:03 +1100 Subject: [PATCH 1/6] feat: add support for instance labels --- google/cloud/spanner_v1/client.py | 2 ++ google/cloud/spanner_v1/instance.py | 17 +++++++++++++++-- tests/unit/test_client.py | 4 ++++ tests/unit/test_instance.py | 22 ++++++++++++++++++++-- 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/google/cloud/spanner_v1/client.py b/google/cloud/spanner_v1/client.py index b433f0c7b0..f2f54a078e 100644 --- a/google/cloud/spanner_v1/client.py +++ b/google/cloud/spanner_v1/client.py @@ -289,6 +289,7 @@ def instance( configuration_name=None, display_name=None, node_count=DEFAULT_NODE_COUNT, + labels=None, ): """Factory to create a instance associated with this client. @@ -323,6 +324,7 @@ def instance( node_count, display_name, self._emulator_host, + labels ) 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 be49dd2d84..134262656a 100644 --- a/google/cloud/spanner_v1/instance.py +++ b/google/cloud/spanner_v1/instance.py @@ -99,6 +99,12 @@ class Instance(object): Cloud Console UI. (Must be between 4 and 30 characters.) If this value is not set in the constructor, will fall back to the instance ID. + + :type emulator_host: str + :param emulator_host: (Optional) + + :type labels: str + :param labels: (Optional) """ def __init__( @@ -109,6 +115,7 @@ def __init__( node_count=DEFAULT_NODE_COUNT, display_name=None, emulator_host=None, + labels=None ): self.instance_id = instance_id self._client = client @@ -116,6 +123,9 @@ def __init__( self.node_count = node_count self.display_name = display_name or instance_id self.emulator_host = emulator_host + if labels is None: + labels = {} + self.labels = labels def _update_from_pb(self, instance_pb): """Refresh self from the server-provided protobuf. @@ -127,6 +137,7 @@ def _update_from_pb(self, instance_pb): self.display_name = instance_pb.display_name self.configuration_name = instance_pb.config self.node_count = instance_pb.node_count + self.labels = instance_pb.labels @classmethod def from_pb(cls, instance_pb, client): @@ -242,6 +253,7 @@ def create(self): config=self.configuration_name, display_name=self.display_name, node_count=self.node_count, + labels=self.labels, ) metadata = _metadata_with_prefix(self.name) @@ -296,7 +308,7 @@ def update(self): .. note:: - Updates the ``display_name`` and ``node_count``. To change those + Updates the ``display_name``, ``node_count`` and ``labels. To change those values before updating, set them via .. code:: python @@ -316,8 +328,9 @@ def update(self): config=self.configuration_name, display_name=self.display_name, node_count=self.node_count, + labels=self.labels, ) - field_mask = FieldMask(paths=["config", "display_name", "node_count"]) + field_mask = FieldMask(paths=["config", "display_name", "node_count", "labels"]) metadata = _metadata_with_prefix(self.name) future = api.update_instance( diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index a3001e61ae..9c260c5f95 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -37,6 +37,7 @@ class TestClient(unittest.TestCase): INSTANCE_NAME = "%s/instances/%s" % (PATH, INSTANCE_ID) DISPLAY_NAME = "display-name" NODE_COUNT = 5 + LABELS = {"test": "true"} TIMEOUT_SECONDS = 80 def _get_target_class(self): @@ -518,6 +519,7 @@ def test_instance_factory_defaults(self): self.assertIsNone(instance.configuration_name) self.assertEqual(instance.display_name, self.INSTANCE_ID) self.assertEqual(instance.node_count, DEFAULT_NODE_COUNT) + self.assertEqual(instance.labels, {}) self.assertIs(instance._client, client) def test_instance_factory_explicit(self): @@ -531,6 +533,7 @@ def test_instance_factory_explicit(self): self.CONFIGURATION_NAME, display_name=self.DISPLAY_NAME, node_count=self.NODE_COUNT, + labels=self.LABELS, ) self.assertIsInstance(instance, Instance) @@ -538,6 +541,7 @@ def test_instance_factory_explicit(self): self.assertEqual(instance.configuration_name, self.CONFIGURATION_NAME) self.assertEqual(instance.display_name, self.DISPLAY_NAME) self.assertEqual(instance.node_count, self.NODE_COUNT) + self.assertEqual(instance.labels, self.LABELS) self.assertIs(instance._client, client) def test_list_instances(self): diff --git a/tests/unit/test_instance.py b/tests/unit/test_instance.py index 0694d438a2..5e3ec1f070 100644 --- a/tests/unit/test_instance.py +++ b/tests/unit/test_instance.py @@ -38,6 +38,7 @@ class TestInstance(unittest.TestCase): TIMEOUT_SECONDS = 1 DATABASE_ID = "database_id" DATABASE_NAME = "%s/databases/%s" % (INSTANCE_NAME, DATABASE_ID) + LABELS = {"test": "true"} def _getTargetClass(self): from google.cloud.spanner_v1.instance import Instance @@ -57,6 +58,7 @@ def test_constructor_defaults(self): self.assertIs(instance.configuration_name, None) self.assertEqual(instance.node_count, DEFAULT_NODE_COUNT) self.assertEqual(instance.display_name, self.INSTANCE_ID) + self.assertEqual(instance.labels, {}) def test_constructor_non_default(self): DISPLAY_NAME = "display_name" @@ -68,12 +70,14 @@ def test_constructor_non_default(self): configuration_name=self.CONFIG_NAME, node_count=self.NODE_COUNT, display_name=DISPLAY_NAME, + labels=self.LABELS, ) self.assertEqual(instance.instance_id, self.INSTANCE_ID) self.assertIs(instance._client, client) self.assertEqual(instance.configuration_name, self.CONFIG_NAME) self.assertEqual(instance.node_count, self.NODE_COUNT) self.assertEqual(instance.display_name, DISPLAY_NAME) + self.assertEqual(instance.labels, self.LABELS) def test_copy(self): DISPLAY_NAME = "display_name" @@ -145,6 +149,7 @@ def test_from_pb_success(self): name=self.INSTANCE_NAME, config=self.CONFIG_NAME, display_name=self.INSTANCE_ID, + labels=self.LABELS, ) klass = self._getTargetClass() @@ -153,6 +158,7 @@ def test_from_pb_success(self): self.assertEqual(instance._client, client) self.assertEqual(instance.instance_id, self.INSTANCE_ID) self.assertEqual(instance.configuration_name, self.CONFIG_NAME) + self.assertEqual(instance.labels, self.LABELS) def test_name_property(self): client = _Client(project=self.PROJECT) @@ -160,6 +166,12 @@ def test_name_property(self): instance = self._make_one(self.INSTANCE_ID, client, self.CONFIG_NAME) self.assertEqual(instance.name, self.INSTANCE_NAME) + def test_labels_property(self): + client = _Client(project=self.PROJECT) + + instance = self._make_one(self.INSTANCE_ID, client, self.CONFIG_NAME, labels=self.LABELS) + self.assertEqual(instance.labels, self.LABELS) + def test___eq__(self): client = object() instance1 = self._make_one(self.INSTANCE_ID, client, self.CONFIG_NAME) @@ -231,6 +243,7 @@ def test_create_success(self): configuration_name=self.CONFIG_NAME, display_name=self.DISPLAY_NAME, node_count=self.NODE_COUNT, + labels=self.LABELS ) future = instance.create() @@ -244,6 +257,7 @@ def test_create_success(self): self.assertEqual(instance.config, self.CONFIG_NAME) self.assertEqual(instance.display_name, self.DISPLAY_NAME) self.assertEqual(instance.node_count, self.NODE_COUNT) + self.assertEqual(instance.labels, self.LABELS) self.assertEqual(metadata, [("google-cloud-resource-prefix", instance.name)]) def test_exists_instance_grpc_error(self): @@ -327,6 +341,7 @@ def test_reload_success(self): config=self.CONFIG_NAME, display_name=self.DISPLAY_NAME, node_count=self.NODE_COUNT, + labels=self.LABELS, ) api = client.instance_admin_api = _FauxInstanceAdminAPI( _get_instance_response=instance_pb @@ -338,6 +353,7 @@ def test_reload_success(self): self.assertEqual(instance.configuration_name, self.CONFIG_NAME) self.assertEqual(instance.node_count, self.NODE_COUNT) self.assertEqual(instance.display_name, self.DISPLAY_NAME) + self.assertEqual(instance.labels, self.LABELS) name, metadata = api._got_instance self.assertEqual(name, self.INSTANCE_NAME) @@ -371,7 +387,7 @@ def test_update_not_found(self): instance.update() instance, field_mask, metadata = api._updated_instance - self.assertEqual(field_mask.paths, ["config", "display_name", "node_count"]) + self.assertEqual(field_mask.paths, ["config", "display_name", "node_count", "labels"]) self.assertEqual(instance.name, self.INSTANCE_NAME) self.assertEqual(instance.config, self.CONFIG_NAME) self.assertEqual(instance.display_name, self.INSTANCE_ID) @@ -390,6 +406,7 @@ def test_update_success(self): configuration_name=self.CONFIG_NAME, node_count=self.NODE_COUNT, display_name=self.DISPLAY_NAME, + labels=self.LABELS ) future = instance.update() @@ -397,11 +414,12 @@ def test_update_success(self): self.assertIs(future, op_future) instance, field_mask, metadata = api._updated_instance - self.assertEqual(field_mask.paths, ["config", "display_name", "node_count"]) + self.assertEqual(field_mask.paths, ["config", "display_name", "node_count", "labels"]) 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.node_count, self.NODE_COUNT) + self.assertEqual(instance.labels, self.LABELS) self.assertEqual(metadata, [("google-cloud-resource-prefix", instance.name)]) def test_delete_grpc_error(self): From e80ef50104372a19727d3feb068d593657e76105 Mon Sep 17 00:00:00 2001 From: larkee Date: Tue, 15 Dec 2020 12:27:48 +1100 Subject: [PATCH 2/6] docs: update parameter docstrings --- google/cloud/spanner_v1/client.py | 3 +++ google/cloud/spanner_v1/instance.py | 7 +++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/google/cloud/spanner_v1/client.py b/google/cloud/spanner_v1/client.py index f2f54a078e..d7bb55f9d9 100644 --- a/google/cloud/spanner_v1/client.py +++ b/google/cloud/spanner_v1/client.py @@ -314,6 +314,9 @@ def instance( :param node_count: (Optional) The number of nodes in the instance's cluster; used to set up the instance's cluster. + :type labels: str + :param labels: (Optional) User-assigned labels 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 134262656a..7caec032eb 100644 --- a/google/cloud/spanner_v1/instance.py +++ b/google/cloud/spanner_v1/instance.py @@ -101,10 +101,13 @@ class Instance(object): constructor, will fall back to the instance ID. :type emulator_host: str - :param emulator_host: (Optional) + :param emulator_host: (Optional) The address for the Cloud Spanner emulator + that the database of this instance should connect to. + If this value is not set, the instance will connect + to the Cloud Spanner endpoint. :type labels: str - :param labels: (Optional) + :param labels: (Optional) User-assigned labels for this instance. """ def __init__( From d9a45bbd758941f01f6ac24647fe60bf034a3390 Mon Sep 17 00:00:00 2001 From: larkee Date: Tue, 15 Dec 2020 13:18:17 +1100 Subject: [PATCH 3/6] docs: add missing literal end-string --- 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 7caec032eb..8f2d2f8caf 100644 --- a/google/cloud/spanner_v1/instance.py +++ b/google/cloud/spanner_v1/instance.py @@ -311,7 +311,7 @@ def update(self): .. note:: - Updates the ``display_name``, ``node_count`` and ``labels. To change those + Updates the ``display_name``, ``node_count`` and ``labels``. To change those values before updating, set them via .. code:: python From ff5df32a378a150569d9dce7a9c98d179109855f Mon Sep 17 00:00:00 2001 From: larkee Date: Tue, 15 Dec 2020 14:02:30 +1100 Subject: [PATCH 4/6] style: fix lint errors --- google/cloud/spanner_v1/client.py | 2 +- google/cloud/spanner_v1/instance.py | 2 +- tests/unit/test_instance.py | 16 +++++++++++----- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/google/cloud/spanner_v1/client.py b/google/cloud/spanner_v1/client.py index d7bb55f9d9..947ee5a4e7 100644 --- a/google/cloud/spanner_v1/client.py +++ b/google/cloud/spanner_v1/client.py @@ -327,7 +327,7 @@ def instance( node_count, display_name, self._emulator_host, - labels + labels, ) 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 8f2d2f8caf..c6f5df637a 100644 --- a/google/cloud/spanner_v1/instance.py +++ b/google/cloud/spanner_v1/instance.py @@ -118,7 +118,7 @@ def __init__( node_count=DEFAULT_NODE_COUNT, display_name=None, emulator_host=None, - labels=None + labels=None, ): self.instance_id = instance_id self._client = client diff --git a/tests/unit/test_instance.py b/tests/unit/test_instance.py index 5e3ec1f070..082ef9e122 100644 --- a/tests/unit/test_instance.py +++ b/tests/unit/test_instance.py @@ -169,7 +169,9 @@ def test_name_property(self): def test_labels_property(self): client = _Client(project=self.PROJECT) - instance = self._make_one(self.INSTANCE_ID, client, self.CONFIG_NAME, labels=self.LABELS) + instance = self._make_one( + self.INSTANCE_ID, client, self.CONFIG_NAME, labels=self.LABELS + ) self.assertEqual(instance.labels, self.LABELS) def test___eq__(self): @@ -243,7 +245,7 @@ def test_create_success(self): configuration_name=self.CONFIG_NAME, display_name=self.DISPLAY_NAME, node_count=self.NODE_COUNT, - labels=self.LABELS + labels=self.LABELS, ) future = instance.create() @@ -387,7 +389,9 @@ def test_update_not_found(self): instance.update() instance, field_mask, metadata = api._updated_instance - self.assertEqual(field_mask.paths, ["config", "display_name", "node_count", "labels"]) + self.assertEqual( + field_mask.paths, ["config", "display_name", "node_count", "labels"] + ) self.assertEqual(instance.name, self.INSTANCE_NAME) self.assertEqual(instance.config, self.CONFIG_NAME) self.assertEqual(instance.display_name, self.INSTANCE_ID) @@ -406,7 +410,7 @@ def test_update_success(self): configuration_name=self.CONFIG_NAME, node_count=self.NODE_COUNT, display_name=self.DISPLAY_NAME, - labels=self.LABELS + labels=self.LABELS, ) future = instance.update() @@ -414,7 +418,9 @@ def test_update_success(self): self.assertIs(future, op_future) instance, field_mask, metadata = api._updated_instance - self.assertEqual(field_mask.paths, ["config", "display_name", "node_count", "labels"]) + self.assertEqual( + field_mask.paths, ["config", "display_name", "node_count", "labels"] + ) self.assertEqual(instance.name, self.INSTANCE_NAME) self.assertEqual(instance.config, self.CONFIG_NAME) self.assertEqual(instance.display_name, self.DISPLAY_NAME) From 021bed94946df91b92c4ab644defab00de123876 Mon Sep 17 00:00:00 2001 From: larkee Date: Tue, 15 Dec 2020 15:47:55 +1100 Subject: [PATCH 5/6] docs: update labels type --- google/cloud/spanner_v1/client.py | 2 +- google/cloud/spanner_v1/instance.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/google/cloud/spanner_v1/client.py b/google/cloud/spanner_v1/client.py index 947ee5a4e7..f4cd6ef910 100644 --- a/google/cloud/spanner_v1/client.py +++ b/google/cloud/spanner_v1/client.py @@ -314,7 +314,7 @@ def instance( :param node_count: (Optional) The number of nodes in the instance's cluster; used to set up the instance's cluster. - :type labels: str + :type labels: dict (str -> str) or None :param labels: (Optional) User-assigned labels for this instance. :rtype: :class:`~google.cloud.spanner_v1.instance.Instance` diff --git a/google/cloud/spanner_v1/instance.py b/google/cloud/spanner_v1/instance.py index c6f5df637a..9a073bac6a 100644 --- a/google/cloud/spanner_v1/instance.py +++ b/google/cloud/spanner_v1/instance.py @@ -106,7 +106,7 @@ class Instance(object): If this value is not set, the instance will connect to the Cloud Spanner endpoint. - :type labels: str + :type labels: dict (str -> str) or None :param labels: (Optional) User-assigned labels for this instance. """ From 72b2dcc398820444ecde7a996f99f34a1cc156f6 Mon Sep 17 00:00:00 2001 From: larkee Date: Tue, 15 Dec 2020 15:51:46 +1100 Subject: [PATCH 6/6] docs: revert emulator_host docstring --- google/cloud/spanner_v1/instance.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/google/cloud/spanner_v1/instance.py b/google/cloud/spanner_v1/instance.py index 9a073bac6a..e6972487a7 100644 --- a/google/cloud/spanner_v1/instance.py +++ b/google/cloud/spanner_v1/instance.py @@ -100,12 +100,6 @@ class Instance(object): characters.) If this value is not set in the constructor, will fall back to the instance ID. - :type emulator_host: str - :param emulator_host: (Optional) The address for the Cloud Spanner emulator - that the database of this instance should connect to. - If this value is not set, the instance will connect - to the Cloud Spanner endpoint. - :type labels: dict (str -> str) or None :param labels: (Optional) User-assigned labels for this instance. """