From 3467d992ed8afa0e33125794b03c4a0e0440e3d3 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 28 Jun 2016 13:09:31 -0400 Subject: [PATCH 01/10] Work around borked gRPC generation for g.longrunning.operations. See reopened #1918. --- gcloud/bigtable/_generated_v2/operations_grpc_pb2.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/gcloud/bigtable/_generated_v2/operations_grpc_pb2.py b/gcloud/bigtable/_generated_v2/operations_grpc_pb2.py index 66491c348817..5723e1d99fe0 100644 --- a/gcloud/bigtable/_generated_v2/operations_grpc_pb2.py +++ b/gcloud/bigtable/_generated_v2/operations_grpc_pb2.py @@ -1,4 +1,12 @@ - +from google.longrunning.operations_pb2 import ( + CancelOperationRequest, + DeleteOperationRequest, + GetOperationRequest, + ListOperationsRequest, + ListOperationsResponse, + Operation, + google_dot_protobuf_dot_empty__pb2, +) from grpc.beta import implementations as beta_implementations from grpc.beta import interfaces as beta_interfaces from grpc.framework.common import cardinality From 784fe01c539a3c2b2aa42617dafd4f2866e3a202 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 28 Jun 2016 13:10:06 -0400 Subject: [PATCH 02/10] Revise Bigtable system tests to fit instance-based patterns. --- system_tests/bigtable.py | 151 ++++++++++++++++++--------------------- 1 file changed, 68 insertions(+), 83 deletions(-) diff --git a/system_tests/bigtable.py b/system_tests/bigtable.py index 3259adea6c15..fc5cc3694e50 100644 --- a/system_tests/bigtable.py +++ b/system_tests/bigtable.py @@ -35,9 +35,8 @@ from system_test_utils import unique_resource_id -CENTRAL_1C_ZONE = 'us-central1-c' -CLUSTER_ID = 'gcloud' + unique_resource_id('-') -CLUSTER_ID = CLUSTER_ID[:30] # Cluster IDs can't exceed 30 chars. +INSTANCE_ID = 'gcloud' + unique_resource_id('-') +#INSTANCE_ID = INSTANCE_ID[:30] # Instance IDs can't exceed 30 chars. TABLE_ID = 'gcloud-python-test-table' COLUMN_FAMILY_ID1 = u'col-fam-id1' COLUMN_FAMILY_ID2 = u'col-fam-id2' @@ -50,13 +49,7 @@ CELL_VAL4 = b'foo' ROW_KEY = b'row-key' ROW_KEY_ALT = b'row-key-alt' -EXISTING_CLUSTERS = [] -EXPECTED_ZONES = ( - 'asia-east1-b', - 'europe-west1-c', - 'us-central1-b', - CENTRAL_1C_ZONE, -) +EXISTING_INSTANCES = [] class Config(object): @@ -66,13 +59,13 @@ class Config(object): global state. """ CLIENT = None - CLUSTER = None + INSTANCE = None def _operation_wait(operation, max_attempts=5): """Wait until an operation has completed. - :type operation: :class:`gcloud.bigtable.cluster.Operation` + :type operation: :class:`gcloud.bigtable.instance.Operation` :param operation: Operation that has not finished. :type max_attempts: int @@ -92,101 +85,93 @@ def _operation_wait(operation, max_attempts=5): def setUpModule(): _helpers.PROJECT = TESTS_PROJECT Config.CLIENT = Client(admin=True) - Config.CLUSTER = Config.CLIENT.cluster(CENTRAL_1C_ZONE, CLUSTER_ID, - display_name=CLUSTER_ID) + Config.INSTANCE = Config.CLIENT.instance(INSTANCE_ID) Config.CLIENT.start() - clusters, failed_zones = Config.CLIENT.list_clusters() + instances, failed_locations = Config.CLIENT.list_instances() - if len(failed_zones) != 0: - raise ValueError('List clusters failed in module set up.') + if len(failed_locations) != 0: + raise ValueError('List instances failed in module set up.') - EXISTING_CLUSTERS[:] = clusters + EXISTING_INSTANCES[:] = instances - # After listing, create the test cluster. - created_op = Config.CLUSTER.create() + # After listing, create the test instance. + created_op = Config.INSTANCE.create() if not _operation_wait(created_op): - raise RuntimeError('Cluster creation exceed 5 seconds.') + raise RuntimeError('Instance creation exceed 5 seconds.') def tearDownModule(): - Config.CLUSTER.delete() + Config.INSTANCE.delete() Config.CLIENT.stop() -class TestClusterAdminAPI(unittest2.TestCase): +class TestInstanceAdminAPI(unittest2.TestCase): def setUp(self): - self.clusters_to_delete = [] + self.instances_to_delete = [] def tearDown(self): - for cluster in self.clusters_to_delete: - cluster.delete() - - def test_list_zones(self): - zones = Config.CLIENT.list_zones() - self.assertEqual(sorted(zones), sorted(EXPECTED_ZONES)) - - def test_list_clusters(self): - clusters, failed_zones = Config.CLIENT.list_clusters() - self.assertEqual(failed_zones, []) - # We have added one new cluster in `setUpModule`. - self.assertEqual(len(clusters), len(EXISTING_CLUSTERS) + 1) - for cluster in clusters: - cluster_existence = (cluster in EXISTING_CLUSTERS or - cluster == Config.CLUSTER) - self.assertTrue(cluster_existence) + for instance in self.instances_to_delete: + instance.delete() + + def test_list_instances(self): + instances, failed_locations = Config.CLIENT.list_instances() + self.assertEqual(failed_locations, []) + # We have added one new instance in `setUpModule`. + self.assertEqual(len(instances), len(EXISTING_INSTANCES) + 1) + for instance in instances: + instance_existence = (instance in EXISTING_INSTANCES or + instance == Config.INSTANCE) + self.assertTrue(instance_existence) def test_reload(self): - # Use same arguments as Config.CLUSTER (created in `setUpModule`) + # Use same arguments as Config.INSTANCE (created in `setUpModule`) # so we can use reload() on a fresh instance. - cluster = Config.CLIENT.cluster(CENTRAL_1C_ZONE, CLUSTER_ID) + instance = Config.CLIENT.instance(INSTANCE_ID) # Make sure metadata unset before reloading. - cluster.display_name = None - cluster.serve_nodes = None + instance.display_name = None - cluster.reload() - self.assertEqual(cluster.display_name, Config.CLUSTER.display_name) - self.assertEqual(cluster.serve_nodes, Config.CLUSTER.serve_nodes) + instance.reload() + self.assertEqual(instance.display_name, Config.INSTANCE.display_name) - def test_create_cluster(self): - cluster_id = 'new' + unique_resource_id('-') - cluster_id = cluster_id[:30] # Cluster IDs can't exceed 30 chars. - cluster = Config.CLIENT.cluster(CENTRAL_1C_ZONE, cluster_id) - operation = cluster.create() - # Make sure this cluster gets deleted after the test case. - self.clusters_to_delete.append(cluster) + def test_create_instance(self): + instance_id = 'new' + unique_resource_id('-') + #instance_id = instance_id[:30] # Instance IDs can't exceed 30 chars. + instance = Config.CLIENT.instance(instance_id) + operation = instance.create() + # Make sure this instance gets deleted after the test case. + self.instances_to_delete.append(instance) # We want to make sure the operation completes. self.assertTrue(_operation_wait(operation)) - # Create a new cluster instance and make sure it is the same. - cluster_alt = Config.CLIENT.cluster(CENTRAL_1C_ZONE, cluster_id) - cluster_alt.reload() + # Create a new instance instance and make sure it is the same. + instance_alt = Config.CLIENT.instance(instance_id) + instance_alt.reload() - self.assertEqual(cluster, cluster_alt) - self.assertEqual(cluster.display_name, cluster_alt.display_name) - self.assertEqual(cluster.serve_nodes, cluster_alt.serve_nodes) + self.assertEqual(instance, instance_alt) + self.assertEqual(instance.display_name, instance_alt.display_name) def test_update(self): - curr_display_name = Config.CLUSTER.display_name - Config.CLUSTER.display_name = 'Foo Bar Baz' - operation = Config.CLUSTER.update() + curr_display_name = Config.INSTANCE.display_name + Config.INSTANCE.display_name = 'Foo Bar Baz' + operation = Config.INSTANCE.update() # We want to make sure the operation completes. self.assertTrue(_operation_wait(operation)) - # Create a new cluster instance and make sure it is the same. - cluster_alt = Config.CLIENT.cluster(CENTRAL_1C_ZONE, CLUSTER_ID) - self.assertNotEqual(cluster_alt.display_name, - Config.CLUSTER.display_name) - cluster_alt.reload() - self.assertEqual(cluster_alt.display_name, - Config.CLUSTER.display_name) + # Create a new instance instance and make sure it is the same. + instance_alt = Config.CLIENT.instance(INSTANCE_ID) + self.assertNotEqual(instance_alt.display_name, + Config.INSTANCE.display_name) + instance_alt.reload() + self.assertEqual(instance_alt.display_name, + Config.INSTANCE.display_name) - # Make sure to put the cluster back the way it was for the + # Make sure to put the instance back the way it was for the # other test cases. - Config.CLUSTER.display_name = curr_display_name - operation = Config.CLUSTER.update() + Config.INSTANCE.display_name = curr_display_name + operation = Config.INSTANCE.update() # We want to make sure the operation completes. self.assertTrue(_operation_wait(operation)) @@ -196,7 +181,7 @@ class TestTableAdminAPI(unittest2.TestCase): @classmethod def setUpClass(cls): - cls._table = Config.CLUSTER.table(TABLE_ID) + cls._table = Config.INSTANCE.table(TABLE_ID) cls._table.create() @classmethod @@ -211,14 +196,14 @@ def tearDown(self): table.delete() def test_list_tables(self): - # Since `Config.CLUSTER` is newly created in `setUpModule`, the table + # Since `Config.INSTANCE` is newly created in `setUpModule`, the table # created in `setUpClass` here will be the only one. - tables = Config.CLUSTER.list_tables() + tables = Config.INSTANCE.list_tables() self.assertEqual(tables, [self._table]) def test_create_table(self): temp_table_id = 'foo-bar-baz-table' - temp_table = Config.CLUSTER.table(temp_table_id) + temp_table = Config.INSTANCE.table(temp_table_id) temp_table.create() self.tables_to_delete.append(temp_table) @@ -226,15 +211,15 @@ def test_create_table(self): name_attr = operator.attrgetter('name') expected_tables = sorted([temp_table, self._table], key=name_attr) - # Then query for the tables in the cluster and sort them by + # Then query for the tables in the instance and sort them by # name as well. - tables = Config.CLUSTER.list_tables() + tables = Config.INSTANCE.list_tables() sorted_tables = sorted(tables, key=name_attr) self.assertEqual(sorted_tables, expected_tables) def test_create_column_family(self): temp_table_id = 'foo-bar-baz-table' - temp_table = Config.CLUSTER.table(temp_table_id) + temp_table = Config.INSTANCE.table(temp_table_id) temp_table.create() self.tables_to_delete.append(temp_table) @@ -255,7 +240,7 @@ def test_create_column_family(self): def test_update_column_family(self): temp_table_id = 'foo-bar-baz-table' - temp_table = Config.CLUSTER.table(temp_table_id) + temp_table = Config.INSTANCE.table(temp_table_id) temp_table.create() self.tables_to_delete.append(temp_table) @@ -278,7 +263,7 @@ def test_update_column_family(self): def test_delete_column_family(self): temp_table_id = 'foo-bar-baz-table' - temp_table = Config.CLUSTER.table(temp_table_id) + temp_table = Config.INSTANCE.table(temp_table_id) temp_table.create() self.tables_to_delete.append(temp_table) @@ -299,7 +284,7 @@ class TestDataAPI(unittest2.TestCase): @classmethod def setUpClass(cls): - cls._table = table = Config.CLUSTER.table(TABLE_ID) + cls._table = table = Config.INSTANCE.table(TABLE_ID) table.create() table.column_family(COLUMN_FAMILY_ID1).create() table.column_family(COLUMN_FAMILY_ID2).create() From 024c7f30e4971be0f69d5c502260c7ecc8d5cb17 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 28 Jun 2016 13:51:48 -0400 Subject: [PATCH 03/10] Plumb cluster 'location'/'serve_nodes' through instance creation. Note that 'location' is a required argument for new instances: we need it to pass to the automatically-created cluster belonging to a newly-created instance. Non-default 'serve_nodes' can be passed in the automatically-created cluster belonging to a newly-created instance. --- gcloud/bigtable/client.py | 16 ++- gcloud/bigtable/instance.py | 28 +++++- gcloud/bigtable/test_client.py | 36 ++++++- gcloud/bigtable/test_instance.py | 166 +++++++++++++++++++++++++------ system_tests/bigtable.py | 53 +++++++--- 5 files changed, 242 insertions(+), 57 deletions(-) diff --git a/gcloud/bigtable/client.py b/gcloud/bigtable/client.py index 1405928df98a..82e96ef7a340 100644 --- a/gcloud/bigtable/client.py +++ b/gcloud/bigtable/client.py @@ -43,6 +43,7 @@ from gcloud.bigtable._generated_v2 import ( operations_grpc_pb2 as operations_grpc_v2_pb2) +from gcloud.bigtable.cluster import DEFAULT_SERVE_NODES from gcloud.bigtable.instance import Instance from gcloud.client import _ClientFactoryMixin from gcloud.client import _ClientProjectMixin @@ -375,22 +376,33 @@ def __exit__(self, exc_type, exc_val, exc_t): """Stops the client as a context manager.""" self.stop() - def instance(self, instance_id, display_name=None): + def instance(self, instance_id, location, + display_name=None, serve_nodes=DEFAULT_SERVE_NODES): """Factory to create a instance associated with this client. :type instance_id: str :param instance_id: The ID of the instance. + :type location: string + :param location: location name, in form + ``projects//locations/``; used to + set up the instance's cluster. + :type display_name: str :param display_name: (Optional) The display name for the instance in the 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 serve_nodes: int + :param serve_nodes: (Optional) The number of nodes in the instance's + cluster; used to set up the instance's cluster. + :rtype: :class:`.Instance` :returns: an instance owned by this client. """ - return Instance(instance_id, self, display_name=display_name) + return Instance(instance_id, self, location, + display_name=display_name, serve_nodes=serve_nodes) def list_instances(self): """List instances owned by the project. diff --git a/gcloud/bigtable/instance.py b/gcloud/bigtable/instance.py index c33d8a64c9e1..a18aaa7e9b26 100644 --- a/gcloud/bigtable/instance.py +++ b/gcloud/bigtable/instance.py @@ -27,9 +27,11 @@ from gcloud.bigtable._generated_v2 import ( bigtable_table_admin_pb2 as table_messages_v2_pb2) from gcloud.bigtable.cluster import Cluster +from gcloud.bigtable.cluster import DEFAULT_SERVE_NODES from gcloud.bigtable.table import Table +_EXISTING_INSTANCE_LOCATION = 'existing instance, location in cluster' _INSTANCE_NAME_RE = re.compile(r'^projects/(?P[^/]+)/' r'instances/(?P[a-z][-a-z0-9]*)$') _OPERATION_NAME_RE = re.compile(r'^operations/projects/([^/]+)/' @@ -53,13 +55,18 @@ def _prepare_create_request(instance): :returns: The CreateInstance request object containing the instance info. """ parent_name = ('projects/' + instance._client.project) - return messages_v2_pb2.CreateInstanceRequest( + message = messages_v2_pb2.CreateInstanceRequest( parent=parent_name, instance_id=instance.instance_id, instance=data_v2_pb2.Instance( display_name=instance.display_name, ), ) + cluster = message.clusters[instance.instance_id] + cluster.name = instance.name + '/clusters/' + instance.instance_id + cluster.location = instance._cluster_location + cluster.serve_nodes = instance._cluster_serve_nodes + return message def _parse_pb_any_to_native(any_val, expected_type=None): @@ -199,17 +206,28 @@ class Instance(object): :param client: The client that owns the instance. Provides authorization and a project ID. + :type location: string + :param location: location name, in form + ``projects//locations/``; used to + set up the instance's cluster. + :type display_name: str :param display_name: (Optional) The display name for the instance in the 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 serve_nodes: int + :param serve_nodes: (Optional) The number of nodes in the instance's + cluster; used to set up the instance's cluster. """ - def __init__(self, instance_id, client, - display_name=None): + def __init__(self, instance_id, client, location, display_name=None, + serve_nodes=DEFAULT_SERVE_NODES): self.instance_id = instance_id self.display_name = display_name or instance_id + self._cluster_location = location + self._cluster_serve_nodes = serve_nodes self._client = client def _update_from_pb(self, instance_pb): @@ -246,8 +264,9 @@ def from_pb(cls, instance_pb, client): if match.group('project') != client.project: raise ValueError('Project ID on instance does not match the ' 'project ID on the client') + instance_id = match.group('instance_id') - result = cls(match.group('instance_id'), client) + result = cls(instance_id, client, _EXISTING_INSTANCE_LOCATION) result._update_from_pb(instance_pb) return result @@ -262,6 +281,7 @@ def copy(self): """ new_client = self._client.copy() return self.__class__(self.instance_id, new_client, + self._cluster_location, display_name=self.display_name) @property diff --git a/gcloud/bigtable/test_client.py b/gcloud/bigtable/test_client.py index 414e1ae5e171..0bbc5f35b3f4 100644 --- a/gcloud/bigtable/test_client.py +++ b/gcloud/bigtable/test_client.py @@ -526,20 +526,47 @@ def test_stop_while_stopped(self): # Make sure the cluster stub did not change. self.assertEqual(client._instance_stub_internal, instance_stub) - def test_instance_factory(self): + def test_instance_factory_defaults(self): + from gcloud.bigtable.cluster import DEFAULT_SERVE_NODES from gcloud.bigtable.instance import Instance PROJECT = 'PROJECT' INSTANCE_ID = 'instance-id' DISPLAY_NAME = 'display-name' + LOCATION = 'projects/' + PROJECT + '/locations/locname' + credentials = _Credentials() + client = self._makeOne(project=PROJECT, credentials=credentials) + + instance = client.instance(INSTANCE_ID, LOCATION, + display_name=DISPLAY_NAME) + + self.assertTrue(isinstance(instance, Instance)) + self.assertEqual(instance.instance_id, INSTANCE_ID) + self.assertEqual(instance.display_name, DISPLAY_NAME) + self.assertEqual(instance._cluster_location, LOCATION) + self.assertEqual(instance._cluster_serve_nodes, DEFAULT_SERVE_NODES) + self.assertTrue(instance._client is client) + + def test_instance_factory_w_explicit_serve_nodes(self): + from gcloud.bigtable.instance import Instance + PROJECT = 'PROJECT' + INSTANCE_ID = 'instance-id' + DISPLAY_NAME = 'display-name' + LOCATION = 'projects/' + PROJECT + '/locations/locname' + SERVE_NODES = 5 credentials = _Credentials() client = self._makeOne(project=PROJECT, credentials=credentials) - instance = client.instance(INSTANCE_ID, display_name=DISPLAY_NAME) + instance = client.instance( + INSTANCE_ID, display_name=DISPLAY_NAME, + location=LOCATION, serve_nodes=SERVE_NODES) + self.assertTrue(isinstance(instance, Instance)) self.assertEqual(instance.instance_id, INSTANCE_ID) self.assertEqual(instance.display_name, DISPLAY_NAME) + self.assertEqual(instance._cluster_location, LOCATION) + self.assertEqual(instance._cluster_serve_nodes, SERVE_NODES) self.assertTrue(instance._client is client) def test_list_instances(self): @@ -549,6 +576,7 @@ def test_list_instances(self): bigtable_instance_admin_pb2 as messages_v2_pb2) from gcloud.bigtable._testing import _FakeStub + LOCATION = 'projects/' + self.PROJECT + '/locations/locname' FAILED_LOCATION = 'FAILED' INSTANCE_ID1 = 'instance-id1' INSTANCE_ID2 = 'instance-id2' @@ -593,8 +621,8 @@ def test_list_instances(self): # Create expected_result. failed_locations = [FAILED_LOCATION] instances = [ - client.instance(INSTANCE_ID1), - client.instance(INSTANCE_ID2), + client.instance(INSTANCE_ID1, LOCATION), + client.instance(INSTANCE_ID2, LOCATION), ] expected_result = (instances, failed_locations) diff --git a/gcloud/bigtable/test_instance.py b/gcloud/bigtable/test_instance.py index 38e9ef959194..25ef8d64c165 100644 --- a/gcloud/bigtable/test_instance.py +++ b/gcloud/bigtable/test_instance.py @@ -86,11 +86,12 @@ def _finished_helper(self, done): from gcloud.bigtable.instance import Instance PROJECT = 'PROJECT' + LOCATION = 'projects/' + PROJECT + '/locations/locname' INSTANCE_ID = 'instance-id' TIMEOUT_SECONDS = 1 client = _Client(PROJECT, timeout_seconds=TIMEOUT_SECONDS) - instance = Instance(INSTANCE_ID, client) + instance = Instance(INSTANCE_ID, client, LOCATION) operation = self._makeOne( self.OP_TYPE, self.OP_ID, self.BEGIN, instance=instance) @@ -135,7 +136,8 @@ class TestInstance(unittest2.TestCase): PROJECT = 'project' INSTANCE_ID = 'instance-id' - INSTANCE_NAME = ('projects/' + PROJECT + '/instances/' + INSTANCE_ID) + INSTANCE_NAME = 'projects/' + PROJECT + '/instances/' + INSTANCE_ID + LOCATION = 'projects/' + PROJECT + '/locations/locname' DISPLAY_NAME = 'display_name' OP_ID = 8915 OP_NAME = ('operations/projects/%s/instances/%soperations/%d' % @@ -152,18 +154,21 @@ def _makeOne(self, *args, **kwargs): return self._getTargetClass()(*args, **kwargs) def test_constructor_defaults(self): - client = object() + from gcloud.bigtable.cluster import DEFAULT_SERVE_NODES - instance = self._makeOne(self.INSTANCE_ID, client) + client = object() + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) self.assertEqual(instance.instance_id, self.INSTANCE_ID) self.assertEqual(instance.display_name, self.INSTANCE_ID) self.assertTrue(instance._client is client) + self.assertEqual(instance._cluster_location, self.LOCATION) + self.assertEqual(instance._cluster_serve_nodes, DEFAULT_SERVE_NODES) def test_constructor_non_default(self): display_name = 'display_name' client = object() - instance = self._makeOne(self.INSTANCE_ID, client, + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION, display_name=display_name) self.assertEqual(instance.instance_id, self.INSTANCE_ID) self.assertEqual(instance.display_name, display_name) @@ -173,7 +178,7 @@ def test_copy(self): display_name = 'display_name' client = _Client(self.PROJECT) - instance = self._makeOne(self.INSTANCE_ID, client, + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION, display_name=display_name) new_instance = instance.copy() @@ -187,7 +192,7 @@ def test_copy(self): def test_table_factory(self): from gcloud.bigtable.table import Table - instance = self._makeOne(self.INSTANCE_ID, None) + instance = self._makeOne(self.INSTANCE_ID, None, self.LOCATION) table = instance.table(self.TABLE_ID) self.assertTrue(isinstance(table, Table)) @@ -203,7 +208,7 @@ def test__update_from_pb_success(self): display_name=display_name, ) - instance = self._makeOne(None, None, None) + instance = self._makeOne(None, None, None, None) self.assertEqual(instance.display_name, None) instance._update_from_pb(instance_pb) self.assertEqual(instance.display_name, display_name) @@ -213,13 +218,14 @@ def test__update_from_pb_no_display_name(self): instance_pb2 as data_v2_pb2) instance_pb = data_v2_pb2.Instance() - instance = self._makeOne(None, None, None) + instance = self._makeOne(None, None, None, None) self.assertEqual(instance.display_name, None) with self.assertRaises(ValueError): instance._update_from_pb(instance_pb) self.assertEqual(instance.display_name, None) def test_from_pb_success(self): + from gcloud.bigtable.instance import _EXISTING_INSTANCE_LOCATION from gcloud.bigtable._generated_v2 import ( instance_pb2 as data_v2_pb2) @@ -235,6 +241,8 @@ def test_from_pb_success(self): self.assertTrue(isinstance(instance, klass)) self.assertEqual(instance._client, client) self.assertEqual(instance.instance_id, self.INSTANCE_ID) + self.assertEqual(instance._cluster_location, + _EXISTING_INSTANCE_LOCATION) def test_from_pb_bad_instance_name(self): from gcloud.bigtable._generated_v2 import ( @@ -265,31 +273,31 @@ def test_from_pb_project_mistmatch(self): def test_name_property(self): client = _Client(project=self.PROJECT) - instance = self._makeOne(self.INSTANCE_ID, client) + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) self.assertEqual(instance.name, self.INSTANCE_NAME) def test___eq__(self): client = object() - instance1 = self._makeOne(self.INSTANCE_ID, client) - instance2 = self._makeOne(self.INSTANCE_ID, client) + instance1 = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) + instance2 = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) self.assertEqual(instance1, instance2) def test___eq__type_differ(self): client = object() - instance1 = self._makeOne(self.INSTANCE_ID, client) + instance1 = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) instance2 = object() self.assertNotEqual(instance1, instance2) def test___ne__same_value(self): client = object() - instance1 = self._makeOne(self.INSTANCE_ID, client) - instance2 = self._makeOne(self.INSTANCE_ID, client) + instance1 = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) + instance2 = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) comparison_val = (instance1 != instance2) self.assertFalse(comparison_val) def test___ne__(self): - instance1 = self._makeOne('instance_id1', 'client1') - instance2 = self._makeOne('instance_id2', 'client2') + instance1 = self._makeOne('instance_id1', 'client1', self.LOCATION) + instance2 = self._makeOne('instance_id2', 'client2', self.LOCATION) self.assertNotEqual(instance1, instance2) def test_reload(self): @@ -300,7 +308,7 @@ def test_reload(self): from gcloud.bigtable._testing import _FakeStub client = _Client(self.PROJECT, timeout_seconds=self.TIMEOUT_SECONDS) - instance = self._makeOne(self.INSTANCE_ID, client) + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) # Create request_pb request_pb = messages_v2_pb.GetInstanceRequest( @@ -340,7 +348,62 @@ def test_create(self): from gcloud.bigtable import instance as MUT client = _Client(self.PROJECT, timeout_seconds=self.TIMEOUT_SECONDS) - instance = self._makeOne(self.INSTANCE_ID, client) + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) + + # Create request_pb. Just a mock since we monkey patch + # _prepare_create_request + request_pb = object() + + # Create response_pb + op_begin = object() + response_pb = operations_pb2.Operation(name=self.OP_NAME) + + # Patch the stub used by the API method. + client._instance_stub = stub = _FakeStub(response_pb) + + # Create expected_result. + expected_result = MUT.Operation('create', self.OP_ID, op_begin, + instance=instance) + + # Create the mocks. + prep_create_called = [] + + def mock_prep_create_req(instance): + prep_create_called.append(instance) + return request_pb + + process_operation_called = [] + + def mock_process_operation(operation_pb): + process_operation_called.append(operation_pb) + return self.OP_ID, op_begin + + # Perform the method and check the result. + with _Monkey(MUT, + _prepare_create_request=mock_prep_create_req, + _process_operation=mock_process_operation): + result = instance.create() + + self.assertEqual(result, expected_result) + self.assertEqual(stub.method_calls, [( + 'CreateInstance', + (request_pb, self.TIMEOUT_SECONDS), + {}, + )]) + self.assertEqual(prep_create_called, [instance]) + self.assertEqual(process_operation_called, [response_pb]) + + def test_create_w_explicit_serve_nodes(self): + from google.longrunning import operations_pb2 + from gcloud._testing import _Monkey + from gcloud.bigtable._testing import _FakeStub + from gcloud.bigtable import instance as MUT + + SERVE_NODES = 5 + + client = _Client(self.PROJECT, timeout_seconds=self.TIMEOUT_SECONDS) + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION, + serve_nodes=SERVE_NODES) # Create request_pb. Just a mock since we monkey patch # _prepare_create_request @@ -391,7 +454,7 @@ def test_update(self): from gcloud.bigtable._testing import _FakeStub client = _Client(self.PROJECT, timeout_seconds=self.TIMEOUT_SECONDS) - instance = self._makeOne(self.INSTANCE_ID, client, + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION, display_name=self.DISPLAY_NAME) # Create request_pb @@ -426,7 +489,7 @@ def test_delete(self): from gcloud.bigtable._testing import _FakeStub client = _Client(self.PROJECT, timeout_seconds=self.TIMEOUT_SECONDS) - instance = self._makeOne(self.INSTANCE_ID, client) + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) # Create request_pb request_pb = messages_v2_pb.DeleteInstanceRequest( @@ -465,7 +528,7 @@ def test_list_clusters(self): SERVE_NODES = 4 client = _Client(self.PROJECT, timeout_seconds=self.TIMEOUT_SECONDS) - instance = self._makeOne(self.INSTANCE_ID, client) + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) CLUSTER_NAME1 = (instance.name + '/clusters/' + CLUSTER_ID1) CLUSTER_NAME2 = (instance.name + '/clusters/' + CLUSTER_ID2) @@ -516,7 +579,7 @@ def _list_tables_helper(self, table_name=None): from gcloud.bigtable._testing import _FakeStub client = _Client(self.PROJECT, timeout_seconds=self.TIMEOUT_SECONDS) - instance = self._makeOne(self.INSTANCE_ID, client) + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) # Create request_ request_pb = table_messages_v1_pb2.ListTablesRequest( @@ -566,32 +629,71 @@ def test_list_tables_failure_name_bad_before(self): class Test__prepare_create_request(unittest2.TestCase): + PROJECT = 'PROJECT' + PARENT = 'projects/' + PROJECT + LOCATION = 'projects/' + PROJECT + '/locations/locname' + INSTANCE_ID = 'instance-id' + INSTANCE_NAME = PARENT + '/instances/' + INSTANCE_ID + CLUSTER_NAME = INSTANCE_NAME + '/clusters/' + INSTANCE_ID - def _callFUT(self, instance): + def _callFUT(self, instance, **kw): from gcloud.bigtable.instance import _prepare_create_request - return _prepare_create_request(instance) + return _prepare_create_request(instance, **kw) - def test_it(self): + def test_w_defaults(self): + from gcloud.bigtable.cluster import DEFAULT_SERVE_NODES from gcloud.bigtable._generated_v2 import ( instance_pb2 as data_v2_pb2) from gcloud.bigtable._generated_v2 import ( bigtable_instance_admin_pb2 as messages_v2_pb) from gcloud.bigtable.instance import Instance - PROJECT = 'PROJECT' - INSTANCE_ID = 'instance-id' + client = _Client(self.PROJECT) + + instance = Instance(self.INSTANCE_ID, client, self.LOCATION) + request_pb = self._callFUT(instance) + self.assertTrue(isinstance(request_pb, + messages_v2_pb.CreateInstanceRequest)) + self.assertEqual(request_pb.instance_id, self.INSTANCE_ID) + self.assertEqual(request_pb.parent, self.PARENT) + self.assertTrue(isinstance(request_pb.instance, data_v2_pb2.Instance)) + self.assertEqual(request_pb.instance.name, u'') + self.assertEqual(request_pb.instance.display_name, self.INSTANCE_ID) + + # An instance must also define a same-named cluster + cluster = request_pb.clusters[self.INSTANCE_ID] + self.assertTrue(isinstance(cluster, data_v2_pb2.Cluster)) + self.assertEqual(cluster.name, self.CLUSTER_NAME) + self.assertEqual(cluster.location, self.LOCATION) + self.assertEqual(cluster.serve_nodes, DEFAULT_SERVE_NODES) + + def test_w_explicit_serve_nodes(self): + from gcloud.bigtable._generated_v2 import ( + instance_pb2 as data_v2_pb2) + from gcloud.bigtable._generated_v2 import ( + bigtable_instance_admin_pb2 as messages_v2_pb) + from gcloud.bigtable.instance import Instance DISPLAY_NAME = u'DISPLAY_NAME' - client = _Client(PROJECT) + SERVE_NODES = 5 + client = _Client(self.PROJECT) + instance = Instance(self.INSTANCE_ID, client, self.LOCATION, + display_name=DISPLAY_NAME, + serve_nodes=SERVE_NODES) - instance = Instance(INSTANCE_ID, client, display_name=DISPLAY_NAME) request_pb = self._callFUT(instance) + self.assertTrue(isinstance(request_pb, messages_v2_pb.CreateInstanceRequest)) - self.assertEqual(request_pb.instance_id, INSTANCE_ID) + self.assertEqual(request_pb.instance_id, self.INSTANCE_ID) self.assertEqual(request_pb.parent, - 'projects/' + PROJECT) + 'projects/' + self.PROJECT) self.assertTrue(isinstance(request_pb.instance, data_v2_pb2.Instance)) self.assertEqual(request_pb.instance.display_name, DISPLAY_NAME) + # An instance must also define a same-named cluster + cluster = request_pb.clusters[self.INSTANCE_ID] + self.assertTrue(isinstance(cluster, data_v2_pb2.Cluster)) + self.assertEqual(cluster.location, self.LOCATION) + self.assertEqual(cluster.serve_nodes, SERVE_NODES) class Test__parse_pb_any_to_native(unittest2.TestCase): diff --git a/system_tests/bigtable.py b/system_tests/bigtable.py index fc5cc3694e50..352bf7365d2e 100644 --- a/system_tests/bigtable.py +++ b/system_tests/bigtable.py @@ -14,6 +14,7 @@ import datetime import operator +import os import time import unittest2 @@ -35,8 +36,8 @@ from system_test_utils import unique_resource_id +LOCATION_ID = 'us-central1-c' INSTANCE_ID = 'gcloud' + unique_resource_id('-') -#INSTANCE_ID = INSTANCE_ID[:30] # Instance IDs can't exceed 30 chars. TABLE_ID = 'gcloud-python-test-table' COLUMN_FAMILY_ID1 = u'col-fam-id1' COLUMN_FAMILY_ID2 = u'col-fam-id2' @@ -60,6 +61,7 @@ class Config(object): """ CLIENT = None INSTANCE = None + LOCATION_NAME = None def _operation_wait(operation, max_attempts=5): @@ -82,12 +84,31 @@ def _operation_wait(operation, max_attempts=5): return True +def _retry_backoof(meth, *args, **kw): + from grpc.beta.interfaces import StatusCode + from grpc.framework.interfaces.face.face import AbortionError + backoff_intervals = [1, 2, 4, 8] + while True: + try: + return meth(*args, **kw) + except AbortionError as error: + if error.code != StatusCode.UNAVAILABLE: + raise + if backoff_intervals: + time.sleep(backoff_intervals.pop(0)) + else: + raise + + def setUpModule(): _helpers.PROJECT = TESTS_PROJECT + PROJECT = os.getenv(TESTS_PROJECT) + Config.LOCATION_NAME = 'projects/%s/locations/%s' % (PROJECT, LOCATION_ID) Config.CLIENT = Client(admin=True) - Config.INSTANCE = Config.CLIENT.instance(INSTANCE_ID) + Config.INSTANCE = Config.CLIENT.instance(INSTANCE_ID, Config.LOCATION_NAME) Config.CLIENT.start() - instances, failed_locations = Config.CLIENT.list_instances() + instances, failed_locations = _retry_backoof( + Config.CLIENT.list_instances) if len(failed_locations) != 0: raise ValueError('List instances failed in module set up.') @@ -121,13 +142,13 @@ def test_list_instances(self): self.assertEqual(len(instances), len(EXISTING_INSTANCES) + 1) for instance in instances: instance_existence = (instance in EXISTING_INSTANCES or - instance == Config.INSTANCE) + instance == Config.INSTANCE) self.assertTrue(instance_existence) def test_reload(self): # Use same arguments as Config.INSTANCE (created in `setUpModule`) # so we can use reload() on a fresh instance. - instance = Config.CLIENT.instance(INSTANCE_ID) + instance = Config.CLIENT.instance(INSTANCE_ID, Config.LOCATION_NAME) # Make sure metadata unset before reloading. instance.display_name = None @@ -135,9 +156,9 @@ def test_reload(self): self.assertEqual(instance.display_name, Config.INSTANCE.display_name) def test_create_instance(self): - instance_id = 'new' + unique_resource_id('-') - #instance_id = instance_id[:30] # Instance IDs can't exceed 30 chars. - instance = Config.CLIENT.instance(instance_id) + ALT_INSTANCE_ID = 'new' + unique_resource_id('-') + instance = Config.CLIENT.instance( + ALT_INSTANCE_ID, Config.LOCATION_NAME) operation = instance.create() # Make sure this instance gets deleted after the test case. self.instances_to_delete.append(instance) @@ -146,31 +167,33 @@ def test_create_instance(self): self.assertTrue(_operation_wait(operation)) # Create a new instance instance and make sure it is the same. - instance_alt = Config.CLIENT.instance(instance_id) + instance_alt = Config.CLIENT.instance(ALT_INSTANCE_ID, + Config.LOCATION_NAME) instance_alt.reload() self.assertEqual(instance, instance_alt) self.assertEqual(instance.display_name, instance_alt.display_name) def test_update(self): - curr_display_name = Config.INSTANCE.display_name - Config.INSTANCE.display_name = 'Foo Bar Baz' + CURR_DISPLAY_NAME = Config.INSTANCE.display_name + NEW_DISPLAY_NAME = 'Foo Bar Baz' + Config.INSTANCE.display_name = NEW_DISPLAY_NAME operation = Config.INSTANCE.update() # We want to make sure the operation completes. self.assertTrue(_operation_wait(operation)) # Create a new instance instance and make sure it is the same. - instance_alt = Config.CLIENT.instance(INSTANCE_ID) + instance_alt = Config.CLIENT.instance(INSTANCE_ID, + Config.LOCATION_NAME) self.assertNotEqual(instance_alt.display_name, Config.INSTANCE.display_name) instance_alt.reload() - self.assertEqual(instance_alt.display_name, - Config.INSTANCE.display_name) + self.assertEqual(instance_alt.display_name, NEW_DISPLAY_NAME) # Make sure to put the instance back the way it was for the # other test cases. - Config.INSTANCE.display_name = curr_display_name + Config.INSTANCE.display_name = CURR_DISPLAY_NAME operation = Config.INSTANCE.update() # We want to make sure the operation completes. From 5e85b5d0873d45a4daec94eeb9e656d71da5cdea Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 28 Jun 2016 15:22:57 -0400 Subject: [PATCH 04/10] CreateInstance operation name includes location. --- gcloud/bigtable/instance.py | 32 ++++++++++------- gcloud/bigtable/test_instance.py | 59 +++++++++++++++++++------------- 2 files changed, 54 insertions(+), 37 deletions(-) diff --git a/gcloud/bigtable/instance.py b/gcloud/bigtable/instance.py index a18aaa7e9b26..ee539a19acfa 100644 --- a/gcloud/bigtable/instance.py +++ b/gcloud/bigtable/instance.py @@ -35,8 +35,9 @@ _INSTANCE_NAME_RE = re.compile(r'^projects/(?P[^/]+)/' r'instances/(?P[a-z][-a-z0-9]*)$') _OPERATION_NAME_RE = re.compile(r'^operations/projects/([^/]+)/' - r'instances/([a-z][-a-z0-9]*)/operations/' - r'(?P\d+)$') + r'instances/([a-z][-a-z0-9]*)/' + r'locations/(?P[a-z][-a-z0-9]*)/' + r'operations/(?P\d+)$') _TYPE_URL_BASE = 'type.googleapis.com/google.bigtable.' _ADMIN_TYPE_URL_BASE = _TYPE_URL_BASE + 'admin.v2.' _INSTANCE_CREATE_METADATA = _ADMIN_TYPE_URL_BASE + 'CreateInstanceMetadata' @@ -98,25 +99,24 @@ def _process_operation(operation_pb): :param operation_pb: The long-running operation response from a Create/Update/Undelete instance request. - :rtype: tuple - :returns: A pair of an integer and datetime stamp. The integer is the ID - of the operation (``operation_id``) and the timestamp when - the create operation began (``operation_begin``). + :rtype: (int, str, datetime) + :returns: (operation_id, location_id, operation_begin). :raises: :class:`ValueError ` if the operation name doesn't match the :data:`_OPERATION_NAME_RE` regex. """ match = _OPERATION_NAME_RE.match(operation_pb.name) if match is None: raise ValueError('Operation name was not in the expected ' - 'format after a instance modification.', + 'format after instance creation.', operation_pb.name) + location_id = match.group('location_id') operation_id = int(match.group('operation_id')) request_metadata = _parse_pb_any_to_native(operation_pb.metadata) operation_begin = _pb_timestamp_to_datetime( request_metadata.request_time) - return operation_id, operation_begin + return operation_id, location_id, operation_begin class Operation(object): @@ -135,14 +135,18 @@ class Operation(object): :type begin: :class:`datetime.datetime` :param begin: The time when the operation was started. + :type location_id: str + :param location_id: ID of the location in which the operation is running + :type instance: :class:`Instance` :param instance: The instance that created the operation. """ - def __init__(self, op_type, op_id, begin, instance=None): + def __init__(self, op_type, op_id, begin, location_id, instance=None): self.op_type = op_type self.op_id = op_id self.begin = begin + self.location_id = location_id self._instance = instance self._complete = False @@ -152,6 +156,7 @@ def __eq__(self, other): return (other.op_type == self.op_type and other.op_id == self.op_id and other.begin == self.begin and + other.location_id == self.location_id and other._instance == self._instance and other._complete == self._complete) @@ -169,8 +174,9 @@ def finished(self): if self._complete: raise ValueError('The operation has completed.') - operation_name = ('operations/' + self._instance.name + - '/operations/%d' % (self.op_id,)) + operation_name = ( + 'operations/%s/locations/%s/operations/%d' % + (self._instance.name, self.location_id, self.op_id)) request_pb = operations_pb2.GetOperationRequest(name=operation_name) # We expect a `google.longrunning.operations_pb2.Operation`. operation_pb = self._instance._client._operations_stub.GetOperation( @@ -352,8 +358,8 @@ def create(self): operation_pb = self._client._instance_stub.CreateInstance( request_pb, self._client.timeout_seconds) - op_id, op_begin = _process_operation(operation_pb) - return Operation('create', op_id, op_begin, instance=self) + op_id, loc_id, op_begin = _process_operation(operation_pb) + return Operation('create', op_id, op_begin, loc_id, instance=self) def update(self): """Update this instance. diff --git a/gcloud/bigtable/test_instance.py b/gcloud/bigtable/test_instance.py index 25ef8d64c165..6dd38d66789c 100644 --- a/gcloud/bigtable/test_instance.py +++ b/gcloud/bigtable/test_instance.py @@ -22,6 +22,7 @@ class TestOperation(unittest2.TestCase): OP_TYPE = 'fake-op' OP_ID = 8915 BEGIN = datetime.datetime(2015, 10, 22, 1, 1) + LOCATION_ID = 'loc-id' def _getTargetClass(self): from gcloud.bigtable.instance import Operation @@ -32,11 +33,13 @@ def _makeOne(self, *args, **kwargs): def _constructor_test_helper(self, instance=None): operation = self._makeOne( - self.OP_TYPE, self.OP_ID, self.BEGIN, instance=instance) + self.OP_TYPE, self.OP_ID, self.BEGIN, self.LOCATION_ID, + instance=instance) self.assertEqual(operation.op_type, self.OP_TYPE) self.assertEqual(operation.op_id, self.OP_ID) self.assertEqual(operation.begin, self.BEGIN) + self.assertEqual(operation.location_id, self.LOCATION_ID) self.assertEqual(operation._instance, instance) self.assertFalse(operation._complete) @@ -50,32 +53,36 @@ def test_constructor_explicit_instance(self): def test___eq__(self): instance = object() operation1 = self._makeOne( - self.OP_TYPE, self.OP_ID, self.BEGIN, instance=instance) + self.OP_TYPE, self.OP_ID, self.BEGIN, self.LOCATION_ID, + instance=instance) operation2 = self._makeOne( - self.OP_TYPE, self.OP_ID, self.BEGIN, instance=instance) + self.OP_TYPE, self.OP_ID, self.BEGIN, self.LOCATION_ID, + instance=instance) self.assertEqual(operation1, operation2) def test___eq__type_differ(self): - operation1 = self._makeOne('foo', 123, None) + operation1 = self._makeOne('foo', 123, None, self.LOCATION_ID) operation2 = object() self.assertNotEqual(operation1, operation2) def test___ne__same_value(self): instance = object() operation1 = self._makeOne( - self.OP_TYPE, self.OP_ID, self.BEGIN, instance=instance) + self.OP_TYPE, self.OP_ID, self.BEGIN, self.LOCATION_ID, + instance=instance) operation2 = self._makeOne( - self.OP_TYPE, self.OP_ID, self.BEGIN, instance=instance) + self.OP_TYPE, self.OP_ID, self.BEGIN, self.LOCATION_ID, + instance=instance) comparison_val = (operation1 != operation2) self.assertFalse(comparison_val) def test___ne__(self): - operation1 = self._makeOne('foo', 123, None) - operation2 = self._makeOne('bar', 456, None) + operation1 = self._makeOne('foo', 123, None, self.LOCATION_ID) + operation2 = self._makeOne('bar', 456, None, self.LOCATION_ID) self.assertNotEqual(operation1, operation2) def test_finished_without_operation(self): - operation = self._makeOne(None, None, None) + operation = self._makeOne(None, None, None, None) operation._complete = True with self.assertRaises(ValueError): operation.finished() @@ -93,11 +100,13 @@ def _finished_helper(self, done): client = _Client(PROJECT, timeout_seconds=TIMEOUT_SECONDS) instance = Instance(INSTANCE_ID, client, LOCATION) operation = self._makeOne( - self.OP_TYPE, self.OP_ID, self.BEGIN, instance=instance) + self.OP_TYPE, self.OP_ID, self.BEGIN, self.LOCATION_ID, + instance=instance) # Create request_pb op_name = ('operations/projects/' + PROJECT + '/instances/' + INSTANCE_ID + + '/locations/' + self.LOCATION_ID + '/operations/%d' % (self.OP_ID,)) request_pb = operations_pb2.GetOperationRequest(name=op_name) @@ -355,15 +364,15 @@ def test_create(self): request_pb = object() # Create response_pb - op_begin = object() + OP_BEGIN = object() response_pb = operations_pb2.Operation(name=self.OP_NAME) # Patch the stub used by the API method. client._instance_stub = stub = _FakeStub(response_pb) # Create expected_result. - expected_result = MUT.Operation('create', self.OP_ID, op_begin, - instance=instance) + expected_result = MUT.Operation('create', self.OP_ID, OP_BEGIN, + self.LOCATION, instance=instance) # Create the mocks. prep_create_called = [] @@ -376,7 +385,7 @@ def mock_prep_create_req(instance): def mock_process_operation(operation_pb): process_operation_called.append(operation_pb) - return self.OP_ID, op_begin + return self.OP_ID, self.LOCATION, OP_BEGIN # Perform the method and check the result. with _Monkey(MUT, @@ -410,15 +419,15 @@ def test_create_w_explicit_serve_nodes(self): request_pb = object() # Create response_pb - op_begin = object() + OP_BEGIN = object() response_pb = operations_pb2.Operation(name=self.OP_NAME) # Patch the stub used by the API method. client._instance_stub = stub = _FakeStub(response_pb) # Create expected_result. - expected_result = MUT.Operation('create', self.OP_ID, op_begin, - instance=instance) + expected_result = MUT.Operation('create', self.OP_ID, OP_BEGIN, + self.LOCATION, instance=instance) # Create the mocks. prep_create_called = [] @@ -431,7 +440,7 @@ def mock_prep_create_req(instance): def mock_process_operation(operation_pb): process_operation_called.append(operation_pb) - return self.OP_ID, op_begin + return self.OP_ID, self.LOCATION, OP_BEGIN # Perform the method and check the result. with _Monkey(MUT, @@ -794,10 +803,11 @@ def test_it(self): PROJECT = 'PROJECT' INSTANCE_ID = 'instance-id' - EXPECTED_OPERATION_ID = 234 + LOCATION_ID = 'location' + OP_ID = 234 OPERATION_NAME = ( - 'operations/projects/%s/instances/%s/operations/%d' % - (PROJECT, INSTANCE_ID, EXPECTED_OPERATION_ID)) + 'operations/projects/%s/instances/%s/locations/%s/operations/%d' % + (PROJECT, INSTANCE_ID, LOCATION_ID, OP_ID)) current_op = operations_pb2.Operation(name=OPERATION_NAME) @@ -819,11 +829,12 @@ def mock_pb_timestamp_to_datetime(timestamp): # Exectute method with mocks in place. with _Monkey(MUT, _parse_pb_any_to_native=mock_parse_pb_any_to_native, _pb_timestamp_to_datetime=mock_pb_timestamp_to_datetime): - operation_id, operation_begin = self._callFUT(current_op) + op_id, loc_id, op_begin = self._callFUT(current_op) # Check outputs. - self.assertEqual(operation_id, EXPECTED_OPERATION_ID) - self.assertTrue(operation_begin is expected_operation_begin) + self.assertEqual(op_id, OP_ID) + self.assertTrue(op_begin is expected_operation_begin) + self.assertEqual(loc_id, LOCATION_ID) # Check mocks were used correctly. self.assertEqual(parse_pb_any_called, [(current_op.metadata, None)]) From d2c9dafd99b60ce923a4772721780127d188ecb1 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 28 Jun 2016 15:48:56 -0400 Subject: [PATCH 05/10] 'Row/ColumnFamily._table' has '_instance'. --- gcloud/bigtable/column_family.py | 6 +++--- gcloud/bigtable/row.py | 6 +++--- gcloud/bigtable/test_column_family.py | 4 ++-- gcloud/bigtable/test_row.py | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/gcloud/bigtable/column_family.py b/gcloud/bigtable/column_family.py index 9088e24a72ab..10127aa74961 100644 --- a/gcloud/bigtable/column_family.py +++ b/gcloud/bigtable/column_family.py @@ -261,7 +261,7 @@ def create(self): id=self.column_family_id, create=column_family, ) - client = self._table._cluster._client + client = self._table._instance._client # We expect a `.table_v2_pb2.ColumnFamily`. We ignore it since the only # data it contains are the GC rule and the column family ID already # stored on this instance. @@ -286,7 +286,7 @@ def update(self): request_pb.modifications.add( id=self.column_family_id, update=column_family) - client = self._table._cluster._client + client = self._table._instance._client # We expect a `.table_v2_pb2.ColumnFamily`. We ignore it since the only # data it contains are the GC rule and the column family ID already # stored on this instance. @@ -300,7 +300,7 @@ def delete(self): request_pb.modifications.add( id=self.column_family_id, drop=True) - client = self._table._cluster._client + client = self._table._instance._client # We expect a `google.protobuf.empty_pb2.Empty` client._table_stub.ModifyColumnFamilies(request_pb, client.timeout_seconds) diff --git a/gcloud/bigtable/row.py b/gcloud/bigtable/row.py index 07cf9e1abec4..845747d41923 100644 --- a/gcloud/bigtable/row.py +++ b/gcloud/bigtable/row.py @@ -395,7 +395,7 @@ def commit(self): mutations=mutations_list, ) # We expect a `google.protobuf.empty_pb2.Empty` - client = self._table._cluster._client + client = self._table._instance._client client._data_stub.MutateRow(request_pb, client.timeout_seconds) self.clear() @@ -512,7 +512,7 @@ def commit(self): false_mutations=false_mutations, ) # We expect a `.messages_v2_pb2.CheckAndMutateRowResponse` - client = self._table._cluster._client + client = self._table._instance._client resp = client._data_stub.CheckAndMutateRow( request_pb, client.timeout_seconds) self.clear() @@ -800,7 +800,7 @@ def commit(self): rules=self._rule_pb_list, ) # We expect a `.data_v2_pb2.Row` - client = self._table._cluster._client + client = self._table._instance._client row_response = client._data_stub.ReadModifyWriteRow( request_pb, client.timeout_seconds) diff --git a/gcloud/bigtable/test_column_family.py b/gcloud/bigtable/test_column_family.py index 77d3f7bfdfd7..d9deaf841fa0 100644 --- a/gcloud/bigtable/test_column_family.py +++ b/gcloud/bigtable/test_column_family.py @@ -650,7 +650,7 @@ def _ColumnFamilyPB(*args, **kw): return table_v2_pb2.ColumnFamily(*args, **kw) -class _Cluster(object): +class _Instance(object): def __init__(self, client=None): self._client = client @@ -666,4 +666,4 @@ class _Table(object): def __init__(self, name, client=None): self.name = name - self._cluster = _Cluster(client) + self._instance = _Instance(client) diff --git a/gcloud/bigtable/test_row.py b/gcloud/bigtable/test_row.py index 2cc7630758d2..b5f486cbec0c 100644 --- a/gcloud/bigtable/test_row.py +++ b/gcloud/bigtable/test_row.py @@ -895,7 +895,7 @@ def __init__(self, timeout_seconds=None): self.timeout_seconds = timeout_seconds -class _Cluster(object): +class _Instance(object): def __init__(self, client=None): self._client = client @@ -905,4 +905,4 @@ class _Table(object): def __init__(self, name, client=None): self.name = name - self._cluster = _Cluster(client) + self._instance = _Instance(client) From a01d02e768816284e8f33baac0a8afbb5d19997c Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 28 Jun 2016 15:49:38 -0400 Subject: [PATCH 06/10] V2 UpdateTable no longer returns operation. --- system_tests/bigtable.py | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/system_tests/bigtable.py b/system_tests/bigtable.py index 352bf7365d2e..c9867f480b98 100644 --- a/system_tests/bigtable.py +++ b/system_tests/bigtable.py @@ -175,29 +175,21 @@ def test_create_instance(self): self.assertEqual(instance.display_name, instance_alt.display_name) def test_update(self): - CURR_DISPLAY_NAME = Config.INSTANCE.display_name + OLD_DISPLAY_NAME = Config.INSTANCE.display_name NEW_DISPLAY_NAME = 'Foo Bar Baz' Config.INSTANCE.display_name = NEW_DISPLAY_NAME - operation = Config.INSTANCE.update() + Config.INSTANCE.update() - # We want to make sure the operation completes. - self.assertTrue(_operation_wait(operation)) - - # Create a new instance instance and make sure it is the same. - instance_alt = Config.CLIENT.instance(INSTANCE_ID, - Config.LOCATION_NAME) - self.assertNotEqual(instance_alt.display_name, - Config.INSTANCE.display_name) + # Create a new instance instance and reload it. + instance_alt = Config.CLIENT.instance(INSTANCE_ID, None) + self.assertNotEqual(instance_alt.display_name, NEW_DISPLAY_NAME) instance_alt.reload() self.assertEqual(instance_alt.display_name, NEW_DISPLAY_NAME) # Make sure to put the instance back the way it was for the # other test cases. - Config.INSTANCE.display_name = CURR_DISPLAY_NAME - operation = Config.INSTANCE.update() - - # We want to make sure the operation completes. - self.assertTrue(_operation_wait(operation)) + Config.INSTANCE.display_name = OLD_DISPLAY_NAME + Config.INSTANCE.update() class TestTableAdminAPI(unittest2.TestCase): From 9bdeef7d6fb29ab76d55b946231dcb858feb0e91 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 28 Jun 2016 16:00:50 -0400 Subject: [PATCH 07/10] 'PartialRowData' no longer keeps 'committed' state. The 'PartialRowsData' has state: 'Table.read_row()' ensures it is in the correct state before emitting the PRD. --- system_tests/bigtable.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/system_tests/bigtable.py b/system_tests/bigtable.py index c9867f480b98..7db32e777602 100644 --- a/system_tests/bigtable.py +++ b/system_tests/bigtable.py @@ -357,7 +357,6 @@ def test_read_row(self): # Read back the contents of the row. partial_row_data = self._table.read_row(ROW_KEY) - self.assertTrue(partial_row_data.committed) self.assertEqual(partial_row_data.row_key, ROW_KEY) # Check the cells match. @@ -440,7 +439,6 @@ def test_read_with_label_applied(self): # Bring our two labeled columns together. row_filter = RowFilterUnion(filters=[chain1, chain2]) partial_row_data = self._table.read_row(ROW_KEY, filter_=row_filter) - self.assertTrue(partial_row_data.committed) self.assertEqual(partial_row_data.row_key, ROW_KEY) cells_returned = partial_row_data.cells From 893d4518010bffe97db2624f064452148bad4dd2 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 28 Jun 2016 16:31:39 -0400 Subject: [PATCH 08/10] Typo fix. Addresses: https://github.com/GoogleCloudPlatform/gcloud-python/pull/1930#discussion_r68834783 --- system_tests/bigtable.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/system_tests/bigtable.py b/system_tests/bigtable.py index 7db32e777602..735d5707f8aa 100644 --- a/system_tests/bigtable.py +++ b/system_tests/bigtable.py @@ -84,7 +84,7 @@ def _operation_wait(operation, max_attempts=5): return True -def _retry_backoof(meth, *args, **kw): +def _retry_backoff(meth, *args, **kw): from grpc.beta.interfaces import StatusCode from grpc.framework.interfaces.face.face import AbortionError backoff_intervals = [1, 2, 4, 8] @@ -107,7 +107,7 @@ def setUpModule(): Config.CLIENT = Client(admin=True) Config.INSTANCE = Config.CLIENT.instance(INSTANCE_ID, Config.LOCATION_NAME) Config.CLIENT.start() - instances, failed_locations = _retry_backoof( + instances, failed_locations = _retry_backoff( Config.CLIENT.list_instances) if len(failed_locations) != 0: From d8a01c2dcb56d1edd5f826cb2cacc9bb6d697f14 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 28 Jun 2016 16:52:48 -0400 Subject: [PATCH 09/10] Switch to passing location ID, rather than full path, to instance. It can synthesize the path using its client's project ID. Addresses: https://github.com/GoogleCloudPlatform/gcloud-python/pull/1930#discussion_r68834504 --- gcloud/bigtable/client.py | 3 +- gcloud/bigtable/instance.py | 23 ++++++----- gcloud/bigtable/test_client.py | 15 +++---- gcloud/bigtable/test_instance.py | 71 ++++++++++++++++---------------- system_tests/bigtable.py | 12 ++---- 5 files changed, 63 insertions(+), 61 deletions(-) diff --git a/gcloud/bigtable/client.py b/gcloud/bigtable/client.py index 82e96ef7a340..cf25d05f2a0d 100644 --- a/gcloud/bigtable/client.py +++ b/gcloud/bigtable/client.py @@ -45,6 +45,7 @@ from gcloud.bigtable.cluster import DEFAULT_SERVE_NODES from gcloud.bigtable.instance import Instance +from gcloud.bigtable.instance import _EXISTING_INSTANCE_LOCATION_ID from gcloud.client import _ClientFactoryMixin from gcloud.client import _ClientProjectMixin from gcloud.credentials import get_credentials @@ -376,7 +377,7 @@ def __exit__(self, exc_type, exc_val, exc_t): """Stops the client as a context manager.""" self.stop() - def instance(self, instance_id, location, + def instance(self, instance_id, location=_EXISTING_INSTANCE_LOCATION_ID, display_name=None, serve_nodes=DEFAULT_SERVE_NODES): """Factory to create a instance associated with this client. diff --git a/gcloud/bigtable/instance.py b/gcloud/bigtable/instance.py index ee539a19acfa..dec6c9029744 100644 --- a/gcloud/bigtable/instance.py +++ b/gcloud/bigtable/instance.py @@ -31,7 +31,7 @@ from gcloud.bigtable.table import Table -_EXISTING_INSTANCE_LOCATION = 'existing instance, location in cluster' +_EXISTING_INSTANCE_LOCATION_ID = 'see-existing-cluster' _INSTANCE_NAME_RE = re.compile(r'^projects/(?P[^/]+)/' r'instances/(?P[a-z][-a-z0-9]*)$') _OPERATION_NAME_RE = re.compile(r'^operations/projects/([^/]+)/' @@ -65,7 +65,8 @@ def _prepare_create_request(instance): ) cluster = message.clusters[instance.instance_id] cluster.name = instance.name + '/clusters/' + instance.instance_id - cluster.location = instance._cluster_location + cluster.location = ( + parent_name + '/locations/' + instance._cluster_location_id) cluster.serve_nodes = instance._cluster_serve_nodes return message @@ -212,10 +213,10 @@ class Instance(object): :param client: The client that owns the instance. Provides authorization and a project ID. - :type location: string - :param location: location name, in form - ``projects//locations/``; used to - set up the instance's cluster. + :type location_id: str + :param location_id: ID of the location in which the instance will be + created. Required for instances which do not yet + exist. :type display_name: str :param display_name: (Optional) The display name for the instance in the @@ -228,11 +229,13 @@ class Instance(object): cluster; used to set up the instance's cluster. """ - def __init__(self, instance_id, client, location, display_name=None, + def __init__(self, instance_id, client, + location_id=_EXISTING_INSTANCE_LOCATION_ID, + display_name=None, serve_nodes=DEFAULT_SERVE_NODES): self.instance_id = instance_id self.display_name = display_name or instance_id - self._cluster_location = location + self._cluster_location_id = location_id self._cluster_serve_nodes = serve_nodes self._client = client @@ -272,7 +275,7 @@ def from_pb(cls, instance_pb, client): 'project ID on the client') instance_id = match.group('instance_id') - result = cls(instance_id, client, _EXISTING_INSTANCE_LOCATION) + result = cls(instance_id, client, _EXISTING_INSTANCE_LOCATION_ID) result._update_from_pb(instance_pb) return result @@ -287,7 +290,7 @@ def copy(self): """ new_client = self._client.copy() return self.__class__(self.instance_id, new_client, - self._cluster_location, + self._cluster_location_id, display_name=self.display_name) @property diff --git a/gcloud/bigtable/test_client.py b/gcloud/bigtable/test_client.py index 0bbc5f35b3f4..dd58af08e025 100644 --- a/gcloud/bigtable/test_client.py +++ b/gcloud/bigtable/test_client.py @@ -529,21 +529,22 @@ def test_stop_while_stopped(self): def test_instance_factory_defaults(self): from gcloud.bigtable.cluster import DEFAULT_SERVE_NODES from gcloud.bigtable.instance import Instance + from gcloud.bigtable.instance import _EXISTING_INSTANCE_LOCATION_ID PROJECT = 'PROJECT' INSTANCE_ID = 'instance-id' DISPLAY_NAME = 'display-name' - LOCATION = 'projects/' + PROJECT + '/locations/locname' + LOCATION_ID = 'locname' credentials = _Credentials() client = self._makeOne(project=PROJECT, credentials=credentials) - instance = client.instance(INSTANCE_ID, LOCATION, - display_name=DISPLAY_NAME) + instance = client.instance(INSTANCE_ID, display_name=DISPLAY_NAME) self.assertTrue(isinstance(instance, Instance)) self.assertEqual(instance.instance_id, INSTANCE_ID) self.assertEqual(instance.display_name, DISPLAY_NAME) - self.assertEqual(instance._cluster_location, LOCATION) + self.assertEqual(instance._cluster_location_id, + _EXISTING_INSTANCE_LOCATION_ID) self.assertEqual(instance._cluster_serve_nodes, DEFAULT_SERVE_NODES) self.assertTrue(instance._client is client) @@ -553,19 +554,19 @@ def test_instance_factory_w_explicit_serve_nodes(self): PROJECT = 'PROJECT' INSTANCE_ID = 'instance-id' DISPLAY_NAME = 'display-name' - LOCATION = 'projects/' + PROJECT + '/locations/locname' + LOCATION_ID = 'locname' SERVE_NODES = 5 credentials = _Credentials() client = self._makeOne(project=PROJECT, credentials=credentials) instance = client.instance( INSTANCE_ID, display_name=DISPLAY_NAME, - location=LOCATION, serve_nodes=SERVE_NODES) + location=LOCATION_ID, serve_nodes=SERVE_NODES) self.assertTrue(isinstance(instance, Instance)) self.assertEqual(instance.instance_id, INSTANCE_ID) self.assertEqual(instance.display_name, DISPLAY_NAME) - self.assertEqual(instance._cluster_location, LOCATION) + self.assertEqual(instance._cluster_location_id, LOCATION_ID) self.assertEqual(instance._cluster_serve_nodes, SERVE_NODES) self.assertTrue(instance._client is client) diff --git a/gcloud/bigtable/test_instance.py b/gcloud/bigtable/test_instance.py index 6dd38d66789c..da8827685292 100644 --- a/gcloud/bigtable/test_instance.py +++ b/gcloud/bigtable/test_instance.py @@ -93,12 +93,11 @@ def _finished_helper(self, done): from gcloud.bigtable.instance import Instance PROJECT = 'PROJECT' - LOCATION = 'projects/' + PROJECT + '/locations/locname' INSTANCE_ID = 'instance-id' TIMEOUT_SECONDS = 1 client = _Client(PROJECT, timeout_seconds=TIMEOUT_SECONDS) - instance = Instance(INSTANCE_ID, client, LOCATION) + instance = Instance(INSTANCE_ID, client, self.LOCATION_ID) operation = self._makeOne( self.OP_TYPE, self.OP_ID, self.BEGIN, self.LOCATION_ID, instance=instance) @@ -146,7 +145,8 @@ class TestInstance(unittest2.TestCase): PROJECT = 'project' INSTANCE_ID = 'instance-id' INSTANCE_NAME = 'projects/' + PROJECT + '/instances/' + INSTANCE_ID - LOCATION = 'projects/' + PROJECT + '/locations/locname' + LOCATION_ID = 'locname' + LOCATION = 'projects/' + PROJECT + '/locations/' + LOCATION_ID DISPLAY_NAME = 'display_name' OP_ID = 8915 OP_NAME = ('operations/projects/%s/instances/%soperations/%d' % @@ -166,18 +166,18 @@ def test_constructor_defaults(self): from gcloud.bigtable.cluster import DEFAULT_SERVE_NODES client = object() - instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION_ID) self.assertEqual(instance.instance_id, self.INSTANCE_ID) self.assertEqual(instance.display_name, self.INSTANCE_ID) self.assertTrue(instance._client is client) - self.assertEqual(instance._cluster_location, self.LOCATION) + self.assertEqual(instance._cluster_location_id, self.LOCATION_ID) self.assertEqual(instance._cluster_serve_nodes, DEFAULT_SERVE_NODES) def test_constructor_non_default(self): display_name = 'display_name' client = object() - instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION, + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION_ID, display_name=display_name) self.assertEqual(instance.instance_id, self.INSTANCE_ID) self.assertEqual(instance.display_name, display_name) @@ -187,7 +187,7 @@ def test_copy(self): display_name = 'display_name' client = _Client(self.PROJECT) - instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION, + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION_ID, display_name=display_name) new_instance = instance.copy() @@ -201,7 +201,7 @@ def test_copy(self): def test_table_factory(self): from gcloud.bigtable.table import Table - instance = self._makeOne(self.INSTANCE_ID, None, self.LOCATION) + instance = self._makeOne(self.INSTANCE_ID, None, self.LOCATION_ID) table = instance.table(self.TABLE_ID) self.assertTrue(isinstance(table, Table)) @@ -234,7 +234,7 @@ def test__update_from_pb_no_display_name(self): self.assertEqual(instance.display_name, None) def test_from_pb_success(self): - from gcloud.bigtable.instance import _EXISTING_INSTANCE_LOCATION + from gcloud.bigtable.instance import _EXISTING_INSTANCE_LOCATION_ID from gcloud.bigtable._generated_v2 import ( instance_pb2 as data_v2_pb2) @@ -250,8 +250,8 @@ def test_from_pb_success(self): self.assertTrue(isinstance(instance, klass)) self.assertEqual(instance._client, client) self.assertEqual(instance.instance_id, self.INSTANCE_ID) - self.assertEqual(instance._cluster_location, - _EXISTING_INSTANCE_LOCATION) + self.assertEqual(instance._cluster_location_id, + _EXISTING_INSTANCE_LOCATION_ID) def test_from_pb_bad_instance_name(self): from gcloud.bigtable._generated_v2 import ( @@ -282,31 +282,31 @@ def test_from_pb_project_mistmatch(self): def test_name_property(self): client = _Client(project=self.PROJECT) - instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION_ID) self.assertEqual(instance.name, self.INSTANCE_NAME) def test___eq__(self): client = object() - instance1 = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) - instance2 = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) + instance1 = self._makeOne(self.INSTANCE_ID, client, self.LOCATION_ID) + instance2 = self._makeOne(self.INSTANCE_ID, client, self.LOCATION_ID) self.assertEqual(instance1, instance2) def test___eq__type_differ(self): client = object() - instance1 = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) + instance1 = self._makeOne(self.INSTANCE_ID, client, self.LOCATION_ID) instance2 = object() self.assertNotEqual(instance1, instance2) def test___ne__same_value(self): client = object() - instance1 = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) - instance2 = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) + instance1 = self._makeOne(self.INSTANCE_ID, client, self.LOCATION_ID) + instance2 = self._makeOne(self.INSTANCE_ID, client, self.LOCATION_ID) comparison_val = (instance1 != instance2) self.assertFalse(comparison_val) def test___ne__(self): - instance1 = self._makeOne('instance_id1', 'client1', self.LOCATION) - instance2 = self._makeOne('instance_id2', 'client2', self.LOCATION) + instance1 = self._makeOne('instance_id1', 'client1', self.LOCATION_ID) + instance2 = self._makeOne('instance_id2', 'client2', self.LOCATION_ID) self.assertNotEqual(instance1, instance2) def test_reload(self): @@ -317,7 +317,7 @@ def test_reload(self): from gcloud.bigtable._testing import _FakeStub client = _Client(self.PROJECT, timeout_seconds=self.TIMEOUT_SECONDS) - instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION_ID) # Create request_pb request_pb = messages_v2_pb.GetInstanceRequest( @@ -357,7 +357,7 @@ def test_create(self): from gcloud.bigtable import instance as MUT client = _Client(self.PROJECT, timeout_seconds=self.TIMEOUT_SECONDS) - instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION_ID) # Create request_pb. Just a mock since we monkey patch # _prepare_create_request @@ -372,7 +372,7 @@ def test_create(self): # Create expected_result. expected_result = MUT.Operation('create', self.OP_ID, OP_BEGIN, - self.LOCATION, instance=instance) + self.LOCATION_ID, instance=instance) # Create the mocks. prep_create_called = [] @@ -385,7 +385,7 @@ def mock_prep_create_req(instance): def mock_process_operation(operation_pb): process_operation_called.append(operation_pb) - return self.OP_ID, self.LOCATION, OP_BEGIN + return self.OP_ID, self.LOCATION_ID, OP_BEGIN # Perform the method and check the result. with _Monkey(MUT, @@ -411,7 +411,7 @@ def test_create_w_explicit_serve_nodes(self): SERVE_NODES = 5 client = _Client(self.PROJECT, timeout_seconds=self.TIMEOUT_SECONDS) - instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION, + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION_ID, serve_nodes=SERVE_NODES) # Create request_pb. Just a mock since we monkey patch @@ -427,7 +427,7 @@ def test_create_w_explicit_serve_nodes(self): # Create expected_result. expected_result = MUT.Operation('create', self.OP_ID, OP_BEGIN, - self.LOCATION, instance=instance) + self.LOCATION_ID, instance=instance) # Create the mocks. prep_create_called = [] @@ -440,7 +440,7 @@ def mock_prep_create_req(instance): def mock_process_operation(operation_pb): process_operation_called.append(operation_pb) - return self.OP_ID, self.LOCATION, OP_BEGIN + return self.OP_ID, self.LOCATION_ID, OP_BEGIN # Perform the method and check the result. with _Monkey(MUT, @@ -463,7 +463,7 @@ def test_update(self): from gcloud.bigtable._testing import _FakeStub client = _Client(self.PROJECT, timeout_seconds=self.TIMEOUT_SECONDS) - instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION, + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION_ID, display_name=self.DISPLAY_NAME) # Create request_pb @@ -498,7 +498,7 @@ def test_delete(self): from gcloud.bigtable._testing import _FakeStub client = _Client(self.PROJECT, timeout_seconds=self.TIMEOUT_SECONDS) - instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION_ID) # Create request_pb request_pb = messages_v2_pb.DeleteInstanceRequest( @@ -537,7 +537,7 @@ def test_list_clusters(self): SERVE_NODES = 4 client = _Client(self.PROJECT, timeout_seconds=self.TIMEOUT_SECONDS) - instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION_ID) CLUSTER_NAME1 = (instance.name + '/clusters/' + CLUSTER_ID1) CLUSTER_NAME2 = (instance.name + '/clusters/' + CLUSTER_ID2) @@ -588,7 +588,7 @@ def _list_tables_helper(self, table_name=None): from gcloud.bigtable._testing import _FakeStub client = _Client(self.PROJECT, timeout_seconds=self.TIMEOUT_SECONDS) - instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION) + instance = self._makeOne(self.INSTANCE_ID, client, self.LOCATION_ID) # Create request_ request_pb = table_messages_v1_pb2.ListTablesRequest( @@ -640,7 +640,8 @@ def test_list_tables_failure_name_bad_before(self): class Test__prepare_create_request(unittest2.TestCase): PROJECT = 'PROJECT' PARENT = 'projects/' + PROJECT - LOCATION = 'projects/' + PROJECT + '/locations/locname' + LOCATION_ID = 'locname' + LOCATION_NAME = 'projects/' + PROJECT + '/locations/' + LOCATION_ID INSTANCE_ID = 'instance-id' INSTANCE_NAME = PARENT + '/instances/' + INSTANCE_ID CLUSTER_NAME = INSTANCE_NAME + '/clusters/' + INSTANCE_ID @@ -659,7 +660,7 @@ def test_w_defaults(self): client = _Client(self.PROJECT) - instance = Instance(self.INSTANCE_ID, client, self.LOCATION) + instance = Instance(self.INSTANCE_ID, client, self.LOCATION_ID) request_pb = self._callFUT(instance) self.assertTrue(isinstance(request_pb, messages_v2_pb.CreateInstanceRequest)) @@ -673,7 +674,7 @@ def test_w_defaults(self): cluster = request_pb.clusters[self.INSTANCE_ID] self.assertTrue(isinstance(cluster, data_v2_pb2.Cluster)) self.assertEqual(cluster.name, self.CLUSTER_NAME) - self.assertEqual(cluster.location, self.LOCATION) + self.assertEqual(cluster.location, self.LOCATION_NAME) self.assertEqual(cluster.serve_nodes, DEFAULT_SERVE_NODES) def test_w_explicit_serve_nodes(self): @@ -685,7 +686,7 @@ def test_w_explicit_serve_nodes(self): DISPLAY_NAME = u'DISPLAY_NAME' SERVE_NODES = 5 client = _Client(self.PROJECT) - instance = Instance(self.INSTANCE_ID, client, self.LOCATION, + instance = Instance(self.INSTANCE_ID, client, self.LOCATION_ID, display_name=DISPLAY_NAME, serve_nodes=SERVE_NODES) @@ -701,7 +702,7 @@ def test_w_explicit_serve_nodes(self): # An instance must also define a same-named cluster cluster = request_pb.clusters[self.INSTANCE_ID] self.assertTrue(isinstance(cluster, data_v2_pb2.Cluster)) - self.assertEqual(cluster.location, self.LOCATION) + self.assertEqual(cluster.location, self.LOCATION_NAME) self.assertEqual(cluster.serve_nodes, SERVE_NODES) diff --git a/system_tests/bigtable.py b/system_tests/bigtable.py index 735d5707f8aa..c7e54016534a 100644 --- a/system_tests/bigtable.py +++ b/system_tests/bigtable.py @@ -61,7 +61,6 @@ class Config(object): """ CLIENT = None INSTANCE = None - LOCATION_NAME = None def _operation_wait(operation, max_attempts=5): @@ -103,9 +102,8 @@ def _retry_backoff(meth, *args, **kw): def setUpModule(): _helpers.PROJECT = TESTS_PROJECT PROJECT = os.getenv(TESTS_PROJECT) - Config.LOCATION_NAME = 'projects/%s/locations/%s' % (PROJECT, LOCATION_ID) Config.CLIENT = Client(admin=True) - Config.INSTANCE = Config.CLIENT.instance(INSTANCE_ID, Config.LOCATION_NAME) + Config.INSTANCE = Config.CLIENT.instance(INSTANCE_ID, LOCATION_ID) Config.CLIENT.start() instances, failed_locations = _retry_backoff( Config.CLIENT.list_instances) @@ -148,7 +146,7 @@ def test_list_instances(self): def test_reload(self): # Use same arguments as Config.INSTANCE (created in `setUpModule`) # so we can use reload() on a fresh instance. - instance = Config.CLIENT.instance(INSTANCE_ID, Config.LOCATION_NAME) + instance = Config.CLIENT.instance(INSTANCE_ID, LOCATION_ID) # Make sure metadata unset before reloading. instance.display_name = None @@ -157,8 +155,7 @@ def test_reload(self): def test_create_instance(self): ALT_INSTANCE_ID = 'new' + unique_resource_id('-') - instance = Config.CLIENT.instance( - ALT_INSTANCE_ID, Config.LOCATION_NAME) + instance = Config.CLIENT.instance(ALT_INSTANCE_ID, LOCATION_ID) operation = instance.create() # Make sure this instance gets deleted after the test case. self.instances_to_delete.append(instance) @@ -167,8 +164,7 @@ def test_create_instance(self): self.assertTrue(_operation_wait(operation)) # Create a new instance instance and make sure it is the same. - instance_alt = Config.CLIENT.instance(ALT_INSTANCE_ID, - Config.LOCATION_NAME) + instance_alt = Config.CLIENT.instance(ALT_INSTANCE_ID, LOCATION_ID) instance_alt.reload() self.assertEqual(instance, instance_alt) From 6f4acedfe7af96f5d7893d2daad9aa23362e9b58 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 28 Jun 2016 18:37:13 -0400 Subject: [PATCH 10/10] Lint fixes. --- gcloud/bigtable/test_client.py | 1 - system_tests/bigtable.py | 2 -- 2 files changed, 3 deletions(-) diff --git a/gcloud/bigtable/test_client.py b/gcloud/bigtable/test_client.py index dd58af08e025..435798ecdf61 100644 --- a/gcloud/bigtable/test_client.py +++ b/gcloud/bigtable/test_client.py @@ -534,7 +534,6 @@ def test_instance_factory_defaults(self): PROJECT = 'PROJECT' INSTANCE_ID = 'instance-id' DISPLAY_NAME = 'display-name' - LOCATION_ID = 'locname' credentials = _Credentials() client = self._makeOne(project=PROJECT, credentials=credentials) diff --git a/system_tests/bigtable.py b/system_tests/bigtable.py index c7e54016534a..6933bc60847c 100644 --- a/system_tests/bigtable.py +++ b/system_tests/bigtable.py @@ -14,7 +14,6 @@ import datetime import operator -import os import time import unittest2 @@ -101,7 +100,6 @@ def _retry_backoff(meth, *args, **kw): def setUpModule(): _helpers.PROJECT = TESTS_PROJECT - PROJECT = os.getenv(TESTS_PROJECT) Config.CLIENT = Client(admin=True) Config.INSTANCE = Config.CLIENT.instance(INSTANCE_ID, LOCATION_ID) Config.CLIENT.start()