From f8f153d6f3002b987d15d9030dcc841acb851271 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Sat, 26 Mar 2016 20:01:50 -0400 Subject: [PATCH 01/12] Allow predefined ACL names from either XML or JSON APIs. Convert names from the XML API to the equivalent JSON names. Fixes #1659. --- gcloud/storage/acl.py | 34 ++++++++++++++++++++++++---------- gcloud/storage/test_acl.py | 22 +++++++++++++++++++++- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/gcloud/storage/acl.py b/gcloud/storage/acl.py index 4ef0aa040899..92a955239fa1 100644 --- a/gcloud/storage/acl.py +++ b/gcloud/storage/acl.py @@ -168,14 +168,24 @@ class ACL(object): _URL_PATH_ELEM = 'acl' _PREDEFINED_QUERY_PARAM = 'predefinedAcl' - _PREDEFINED_ACLS = frozenset([ + PREDEFINED_XML_ACLS = { + # XML API name -> JSON API name + 'project-private': 'projectPrivate', + 'public-read': 'publicRead', + 'public-read-write': 'publicReadWrite', + 'authenticated-read': 'authenticatedRead', + 'bucket-owner-read': 'bucketOwnerRead', + 'bucket-owner-full-control': 'bucketOwnerFullControl', + } + + PREDEFINED_JSON_ACLS = frozenset([ 'private', - 'project-private', - 'public-read', - 'public-read-write', - 'authenticated-read', - 'bucket-owner-read', - 'bucket-owner-full-control', + 'projectPrivate', + 'publicRead', + 'publicReadWrite', + 'authenticatedRead', + 'bucketOwnerRead', + 'bucketOwnerFullControl', ]) """See: https://cloud.google.com/storage/docs/access-control#predefined-acl @@ -409,7 +419,7 @@ def _save(self, acl, predefined, client): :type predefined: string or None :param predefined: An identifier for a predefined ACL. Must be one - of the keys in :attr:`_PREDEFINED_ACLS` + of the keys in :attr:`PREDEFINED_JSON_ACLS` If passed, `acl` must be None. :type client: :class:`gcloud.storage.client.Client` or ``NoneType`` @@ -458,14 +468,18 @@ def save_predefined(self, predefined, client=None): :type predefined: string :param predefined: An identifier for a predefined ACL. Must be one - of the keys in :attr:`_PREDEFINED_ACLS` + of the keys in :attr:`PREDEFINED_JSON_ACLS` + or :attr:`PREDEFINED_XML_ACLS` (which will be + aliased to the corresponding JSON name). If passed, `acl` must be None. :type client: :class:`gcloud.storage.client.Client` or ``NoneType`` :param client: Optional. The client to use. If not passed, falls back to the ``client`` stored on the ACL's parent. """ - if predefined not in self._PREDEFINED_ACLS: + predefined = self.PREDEFINED_XML_ACLS.get(predefined, predefined) + + if predefined not in self.PREDEFINED_JSON_ACLS: raise ValueError("Invalid predefined ACL: %s" % (predefined,)) self._save(None, predefined, client) diff --git a/gcloud/storage/test_acl.py b/gcloud/storage/test_acl.py index 7ca42b6c1941..2fed5213bb27 100644 --- a/gcloud/storage/test_acl.py +++ b/gcloud/storage/test_acl.py @@ -648,9 +648,29 @@ def test_save_predefined_valid(self): self.assertEqual(kw[0]['query_params'], {'projection': 'full', 'predefinedAcl': PREDEFINED}) + def test_save_predefined_w_XML_alias(self): + PREDEFINED_XML = 'project-private' + PREDEFINED_JSON = 'projectPrivate' + connection = _Connection({'acl': []}) + client = _Client(connection) + acl = self._makeOne() + acl.save_path = '/testing' + acl.loaded = True + acl.save_predefined(PREDEFINED_XML, client=client) + entries = list(acl) + self.assertEqual(len(entries), 0) + kw = connection._requested + self.assertEqual(len(kw), 1) + self.assertEqual(kw[0]['method'], 'PATCH') + self.assertEqual(kw[0]['path'], '/testing') + self.assertEqual(kw[0]['data'], {'acl': []}) + self.assertEqual(kw[0]['query_params'], + {'projection': 'full', + 'predefinedAcl': PREDEFINED_JSON}) + def test_save_predefined_valid_w_alternate_query_param(self): # Cover case where subclass overrides _PREDEFINED_QUERY_PARAM - PREDEFINED = 'private' + PREDEFINED = 'publicRead' connection = _Connection({'acl': []}) client = _Client(connection) acl = self._makeOne() From c0992ac192c202265daa7dab03abb0009d946afe Mon Sep 17 00:00:00 2001 From: Bernard Ojengwa Date: Wed, 23 Mar 2016 16:56:22 +0100 Subject: [PATCH 02/12] Fix: Error Calling DNS#QOUTAS Issue: #1650 fix failing tests remove typing from expected value remove schema marker from dns#quotas return value update test specs for dns.Client#quotas remove schema marker from dns#quotas return value update test specs for dns.Client#quotas remove schema marker from dns#quotas return value fix failing tests for dns#quotas return value conditional skipping of the `kind` key --- gcloud/dns/client.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gcloud/dns/client.py b/gcloud/dns/client.py index b6a692aa01c0..ddc4ded769be 100644 --- a/gcloud/dns/client.py +++ b/gcloud/dns/client.py @@ -55,8 +55,9 @@ def quotas(self): """ path = '/projects/%s' % (self.project,) resp = self.connection.api_request(method='GET', path=path) + return dict([(key, int(value)) - for key, value in resp['quota'].items()]) + for key, value in resp['quota'].items() if key != 'kind']) def list_zones(self, max_results=None, page_token=None): """List zones for the project associated with this client. From 247e2934e15dbefacae93b378e12217d8907795c Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 29 Mar 2016 11:43:11 -0400 Subject: [PATCH 03/12] Add a unit test which actually exercises #1650. --- gcloud/dns/test_client.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/gcloud/dns/test_client.py b/gcloud/dns/test_client.py index 8f59e989c55f..44b168f82efe 100644 --- a/gcloud/dns/test_client.py +++ b/gcloud/dns/test_client.py @@ -68,6 +68,42 @@ def test_quotas_defaults(self): self.assertEqual(req['method'], 'GET') self.assertEqual(req['path'], '/%s' % PATH) + def test_quotas_w_kind_key(self): + PROJECT = 'PROJECT' + PATH = 'projects/%s' % PROJECT + MANAGED_ZONES = 1234 + RRS_PER_RRSET = 23 + RRSETS_PER_ZONE = 345 + RRSET_ADDITIONS = 456 + RRSET_DELETIONS = 567 + TOTAL_SIZE = 67890 + DATA = { + 'quota': { + 'managedZones': str(MANAGED_ZONES), + 'resourceRecordsPerRrset': str(RRS_PER_RRSET), + 'rrsetsPerManagedZone': str(RRSETS_PER_ZONE), + 'rrsetAdditionsPerChange': str(RRSET_ADDITIONS), + 'rrsetDeletionsPerChange': str(RRSET_DELETIONS), + 'totalRrdataSizePerChange': str(TOTAL_SIZE), + } + } + CONVERTED = dict([(key, int(value)) + for key, value in DATA['quota'].items()]) + WITH_KIND = {'quota': DATA['quota'].copy()} + WITH_KIND['quota']['kind'] = 'dns#quota' + creds = _Credentials() + client = self._makeOne(PROJECT, creds) + conn = client.connection = _Connection(WITH_KIND) + + quotas = client.quotas() + + self.assertEqual(quotas, CONVERTED) + + self.assertEqual(len(conn._requested), 1) + req = conn._requested[0] + self.assertEqual(req['method'], 'GET') + self.assertEqual(req['path'], '/%s' % PATH) + def test_list_zones_defaults(self): from gcloud.dns.zone import ManagedZone PROJECT = 'PROJECT' From 21cfae62a130f9206da3223a06e2e3befc78c826 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 23 Mar 2016 12:37:54 -0400 Subject: [PATCH 04/12] Add system tests for topic/subscription IAM policy get/set methods. --- docs/pubsub-usage.rst | 12 ++++----- gcloud/pubsub/iam.py | 12 ++++----- gcloud/pubsub/subscription.py | 3 ++- gcloud/pubsub/test_iam.py | 12 ++++----- gcloud/pubsub/test_subscription.py | 20 +++++++------- gcloud/pubsub/test_topic.py | 24 ++++++++--------- gcloud/pubsub/topic.py | 3 ++- system_tests/pubsub.py | 43 +++++++++++++++++++++++++++--- 8 files changed, 84 insertions(+), 45 deletions(-) diff --git a/docs/pubsub-usage.rst b/docs/pubsub-usage.rst index df58f7273ae6..2f74fd584120 100644 --- a/docs/pubsub-usage.rst +++ b/docs/pubsub-usage.rst @@ -99,12 +99,12 @@ Test permissions allowed by the current IAM policy on a topic: .. doctest:: >>> from gcloud import pubsub - >>> from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + >>> from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE >>> client = pubsub.Client() >>> topic = client.topic('topic_name') >>> allowed = topic.check_iam_permissions( - ... [READER_ROLE, WRITER_ROLE, OWNER_ROLE]) # API request - >>> allowed == [READER_ROLE, WRITER_ROLE] + ... [VIEWER_ROLE, EDITOR_ROLE, OWNER_ROLE]) # API request + >>> allowed == [VIEWER_ROLE, EDITOR_ROLE] True @@ -349,11 +349,11 @@ Test permissions allowed by the current IAM policy on a subscription: .. doctest:: >>> from gcloud import pubsub - >>> from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + >>> from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE >>> client = pubsub.Client() >>> topic = client.topic('topic_name') >>> subscription = topic.subscription('subscription_name') >>> allowed = subscription.check_iam_permissions( - ... [READER_ROLE, WRITER_ROLE, OWNER_ROLE]) # API request - >>> allowed == [READER_ROLE, WRITER_ROLE] + ... [VIEWER_ROLE, EDITOR_ROLE, OWNER_ROLE]) # API request + >>> allowed == [VIEWER_ROLE, EDITOR_ROLE] True diff --git a/gcloud/pubsub/iam.py b/gcloud/pubsub/iam.py index 1ead9d05ce92..064acbb48682 100644 --- a/gcloud/pubsub/iam.py +++ b/gcloud/pubsub/iam.py @@ -16,10 +16,10 @@ OWNER_ROLE = 'roles/owner' """IAM permission implying all rights to an object.""" -WRITER_ROLE = 'roles/writer' +EDITOR_ROLE = 'roles/editor' """IAM permission implying rights to modify an object.""" -READER_ROLE = 'roles/reader' +VIEWER_ROLE = 'roles/viewer' """IAM permission implying rights to access an object without modifying it.""" @@ -127,9 +127,9 @@ def from_api_repr(cls, resource): members = set(binding['members']) if role == OWNER_ROLE: policy.owners = members - elif role == WRITER_ROLE: + elif role == EDITOR_ROLE: policy.writers = members - elif role == READER_ROLE: + elif role == VIEWER_ROLE: policy.readers = members else: raise ValueError('Unknown role: %s' % (role,)) @@ -157,11 +157,11 @@ def to_api_repr(self): if self.writers: bindings.append( - {'role': WRITER_ROLE, 'members': sorted(self.writers)}) + {'role': EDITOR_ROLE, 'members': sorted(self.writers)}) if self.readers: bindings.append( - {'role': READER_ROLE, 'members': sorted(self.readers)}) + {'role': VIEWER_ROLE, 'members': sorted(self.readers)}) if bindings: resource['bindings'] = bindings diff --git a/gcloud/pubsub/subscription.py b/gcloud/pubsub/subscription.py index b9e5ef30834b..fc058c948347 100644 --- a/gcloud/pubsub/subscription.py +++ b/gcloud/pubsub/subscription.py @@ -305,8 +305,9 @@ def set_iam_policy(self, policy, client=None): client = self._require_client(client) path = '%s:setIamPolicy' % (self.path,) resource = policy.to_api_repr() + wrapped = {'policy': resource} resp = client.connection.api_request( - method='POST', path=path, data=resource) + method='POST', path=path, data=wrapped) return Policy.from_api_repr(resp) def check_iam_permissions(self, permissions, client=None): diff --git a/gcloud/pubsub/test_iam.py b/gcloud/pubsub/test_iam.py index d6a6e165e715..dfd0087d8ba0 100644 --- a/gcloud/pubsub/test_iam.py +++ b/gcloud/pubsub/test_iam.py @@ -87,7 +87,7 @@ def test_from_api_repr_only_etag(self): self.assertEqual(list(policy.readers), []) def test_from_api_repr_complete(self): - from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE OWNER1 = 'user:phred@example.com' OWNER2 = 'group:cloud-logs@google.com' WRITER1 = 'domain:google.com' @@ -99,8 +99,8 @@ def test_from_api_repr_complete(self): 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': WRITER_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': READER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, + {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, ], } klass = self._getTargetClass() @@ -134,7 +134,7 @@ def test_to_api_repr_only_etag(self): self.assertEqual(policy.to_api_repr(), {'etag': 'DEADBEEF'}) def test_to_api_repr_full(self): - from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE OWNER1 = 'group:cloud-logs@google.com' OWNER2 = 'user:phred@example.com' WRITER1 = 'domain:google.com' @@ -146,8 +146,8 @@ def test_to_api_repr_full(self): 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': WRITER_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': READER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, + {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, ], } policy = self._makeOne('DEADBEEF', 17) diff --git a/gcloud/pubsub/test_subscription.py b/gcloud/pubsub/test_subscription.py index d4d3b2746e89..de1ac8764e28 100644 --- a/gcloud/pubsub/test_subscription.py +++ b/gcloud/pubsub/test_subscription.py @@ -485,7 +485,7 @@ def test_delete_w_alternate_client(self): self.assertEqual(req['path'], '/%s' % SUB_PATH) def test_get_iam_policy_w_bound_client(self): - from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE OWNER1 = 'user:phred@example.com' OWNER2 = 'group:cloud-logs@google.com' WRITER1 = 'domain:google.com' @@ -497,8 +497,8 @@ def test_get_iam_policy_w_bound_client(self): 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': WRITER_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': READER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, + {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, ], } PROJECT = 'PROJECT' @@ -557,7 +557,7 @@ def test_get_iam_policy_w_alternate_client(self): self.assertEqual(req['path'], '/%s' % PATH) def test_set_iam_policy_w_bound_client(self): - from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE from gcloud.pubsub.iam import Policy OWNER1 = 'group:cloud-logs@google.com' OWNER2 = 'user:phred@example.com' @@ -570,8 +570,8 @@ def test_set_iam_policy_w_bound_client(self): 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': WRITER_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': READER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, + {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, ], } RESPONSE = POLICY.copy() @@ -607,7 +607,7 @@ def test_set_iam_policy_w_bound_client(self): req = conn._requested[0] self.assertEqual(req['method'], 'POST') self.assertEqual(req['path'], '/%s' % PATH) - self.assertEqual(req['data'], POLICY) + self.assertEqual(req['data'], {'policy': POLICY}) def test_set_iam_policy_w_alternate_client(self): from gcloud.pubsub.iam import Policy @@ -639,7 +639,7 @@ def test_set_iam_policy_w_alternate_client(self): req = conn2._requested[0] self.assertEqual(req['method'], 'POST') self.assertEqual(req['path'], '/%s' % PATH) - self.assertEqual(req['data'], {}) + self.assertEqual(req['data'], {'policy': {}}) def test_check_iam_permissions_w_bound_client(self): PROJECT = 'PROJECT' @@ -647,7 +647,7 @@ def test_check_iam_permissions_w_bound_client(self): SUB_NAME = 'sub_name' PATH = 'projects/%s/subscriptions/%s:testIamPermissions' % ( PROJECT, SUB_NAME) - ROLES = ['roles/reader', 'roles/writer', 'roles/owner'] + ROLES = ['roles/viewer', 'roles/editor', 'roles/owner'] REQUESTED = { 'permissions': ROLES, } @@ -674,7 +674,7 @@ def test_check_iam_permissions_w_alternate_client(self): SUB_NAME = 'sub_name' PATH = 'projects/%s/subscriptions/%s:testIamPermissions' % ( PROJECT, SUB_NAME) - ROLES = ['roles/reader', 'roles/writer', 'roles/owner'] + ROLES = ['roles/viewer', 'roles/editor', 'roles/owner'] REQUESTED = { 'permissions': ROLES, } diff --git a/gcloud/pubsub/test_topic.py b/gcloud/pubsub/test_topic.py index de52cddc1c7f..96830c789bb6 100644 --- a/gcloud/pubsub/test_topic.py +++ b/gcloud/pubsub/test_topic.py @@ -453,7 +453,7 @@ def test_list_subscriptions_missing_key(self): self.assertEqual(req['query_params'], {}) def test_get_iam_policy_w_bound_client(self): - from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE OWNER1 = 'user:phred@example.com' OWNER2 = 'group:cloud-logs@google.com' WRITER1 = 'domain:google.com' @@ -465,8 +465,8 @@ def test_get_iam_policy_w_bound_client(self): 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': WRITER_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': READER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, + {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, ], } TOPIC_NAME = 'topic_name' @@ -522,7 +522,7 @@ def test_get_iam_policy_w_alternate_client(self): def test_set_iam_policy_w_bound_client(self): from gcloud.pubsub.iam import Policy - from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE OWNER1 = 'group:cloud-logs@google.com' OWNER2 = 'user:phred@example.com' WRITER1 = 'domain:google.com' @@ -534,8 +534,8 @@ def test_set_iam_policy_w_bound_client(self): 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': WRITER_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': READER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, + {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, ], } RESPONSE = POLICY.copy() @@ -569,7 +569,7 @@ def test_set_iam_policy_w_bound_client(self): req = conn._requested[0] self.assertEqual(req['method'], 'POST') self.assertEqual(req['path'], '/%s' % PATH) - self.assertEqual(req['data'], POLICY) + self.assertEqual(req['data'], {'policy': POLICY}) def test_set_iam_policy_w_alternate_client(self): from gcloud.pubsub.iam import Policy @@ -599,15 +599,15 @@ def test_set_iam_policy_w_alternate_client(self): req = conn2._requested[0] self.assertEqual(req['method'], 'POST') self.assertEqual(req['path'], '/%s' % PATH) - self.assertEqual(req['data'], {}) + self.assertEqual(req['data'], {'policy': {}}) def test_check_iam_permissions_w_bound_client(self): - from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE TOPIC_NAME = 'topic_name' PROJECT = 'PROJECT' PATH = 'projects/%s/topics/%s:testIamPermissions' % ( PROJECT, TOPIC_NAME) - ROLES = [READER_ROLE, WRITER_ROLE, OWNER_ROLE] + ROLES = [VIEWER_ROLE, EDITOR_ROLE, OWNER_ROLE] REQUESTED = { 'permissions': ROLES, } @@ -628,12 +628,12 @@ def test_check_iam_permissions_w_bound_client(self): self.assertEqual(req['data'], REQUESTED) def test_check_iam_permissions_w_alternate_client(self): - from gcloud.pubsub.iam import OWNER_ROLE, WRITER_ROLE, READER_ROLE + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE TOPIC_NAME = 'topic_name' PROJECT = 'PROJECT' PATH = 'projects/%s/topics/%s:testIamPermissions' % ( PROJECT, TOPIC_NAME) - ROLES = [READER_ROLE, WRITER_ROLE, OWNER_ROLE] + ROLES = [VIEWER_ROLE, EDITOR_ROLE, OWNER_ROLE] REQUESTED = { 'permissions': ROLES, } diff --git a/gcloud/pubsub/topic.py b/gcloud/pubsub/topic.py index 859a84c93d29..5e442afeeb59 100644 --- a/gcloud/pubsub/topic.py +++ b/gcloud/pubsub/topic.py @@ -299,8 +299,9 @@ def set_iam_policy(self, policy, client=None): client = self._require_client(client) path = '%s:setIamPolicy' % (self.path,) resource = policy.to_api_repr() + wrapped = {'policy': resource} resp = client.connection.api_request( - method='POST', path=path, data=resource) + method='POST', path=path, data=wrapped) return Policy.from_api_repr(resp) def check_iam_permissions(self, permissions, client=None): diff --git a/system_tests/pubsub.py b/system_tests/pubsub.py index 956a788c6d36..cce3e0118d04 100644 --- a/system_tests/pubsub.py +++ b/system_tests/pubsub.py @@ -89,7 +89,7 @@ def test_create_subscription_defaults(self): self.assertFalse(topic.exists()) topic.create() self.to_delete.append(topic) - SUBSCRIPTION_NAME = 'subscribing-now' + SUBSCRIPTION_NAME = 'subscribing-now-%d' % (1000 * time.time(),) subscription = topic.subscription(SUBSCRIPTION_NAME) self.assertFalse(subscription.exists()) subscription.create() @@ -103,7 +103,7 @@ def test_create_subscription_w_ack_deadline(self): self.assertFalse(topic.exists()) topic.create() self.to_delete.append(topic) - SUBSCRIPTION_NAME = 'subscribing-now' + SUBSCRIPTION_NAME = 'subscribing-now-%d' % (1000 * time.time(),) subscription = topic.subscription(SUBSCRIPTION_NAME, ack_deadline=120) self.assertFalse(subscription.exists()) subscription.create() @@ -142,7 +142,7 @@ def test_message_pull_mode_e2e(self): self.assertFalse(topic.exists()) topic.create() self.to_delete.append(topic) - SUBSCRIPTION_NAME = 'subscribing-now' + SUBSCRIPTION_NAME = 'subscribing-now-%d' % (1000 * time.time(),) subscription = topic.subscription(SUBSCRIPTION_NAME) self.assertFalse(subscription.exists()) subscription.create() @@ -168,3 +168,40 @@ def _by_timestamp(message): self.assertEqual(message1.attributes['extra'], EXTRA_1) self.assertEqual(message2.data, MESSAGE_2) self.assertEqual(message2.attributes['extra'], EXTRA_2) + + def test_topic_iam_policy(self): + topic_name = 'test-topic-iam-policy-topic-%d' % (1000 * time.time(),) + topic = Config.CLIENT.topic(topic_name) + topic.create() + count = 5 + while count > 0 and not topic.exists(): + time.sleep(1) + count -= 1 + self.to_delete.append(topic) + policy = topic.get_iam_policy() + policy.readers.add(policy.user('jjg@google.com')) + new_policy = topic.set_iam_policy(policy) + self.assertEqual(new_policy.readers, policy.readers) + + def test_subscription_iam_policy(self): + topic_name = 'test-sub-iam-policy-topic-%d' % (1000 * time.time(),) + topic = Config.CLIENT.topic(topic_name) + topic.create() + count = 5 + while count > 0 and not topic.exists(): + time.sleep(1) + count -= 1 + self.assertTrue(topic.exists()) + self.to_delete.append(topic) + SUB_NAME = 'test-sub-iam-policy-sub-%d' % (1000 * time.time(),) + subscription = topic.subscription(SUB_NAME) + subscription.create() + count = 5 + while count > 0 and not subscription.exists(): + time.sleep(1) + count -= 1 + self.to_delete.insert(0, subscription) + policy = subscription.get_iam_policy() + policy.readers.add(policy.user('jjg@google.com')) + new_policy = subscription.set_iam_policy(policy) + self.assertEqual(new_policy.readers, policy.readers) From b53fec52fff7ae3dd5b230bfff2b4e8f66c0573c Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 29 Mar 2016 13:59:03 -0400 Subject: [PATCH 05/12] Cache job constructed for a synchronous query. Fixes #1673. --- gcloud/bigquery/query.py | 10 +++++++--- gcloud/bigquery/test_query.py | 2 ++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/gcloud/bigquery/query.py b/gcloud/bigquery/query.py index 0e580cb4826f..f9158703ce68 100644 --- a/gcloud/bigquery/query.py +++ b/gcloud/bigquery/query.py @@ -50,6 +50,7 @@ def __init__(self, query, client): self._properties = {} self.query = query self._configuration = _SyncQueryConfiguration() + self._job = None @property def project(self): @@ -134,9 +135,12 @@ def job(self): :returns: Job instance used to run the query (None until ``jobReference`` property is set by the server). """ - job_ref = self._properties.get('jobReference') - if job_ref is not None: - return QueryJob(job_ref['jobId'], self.query, self._client) + if self._job is None: + job_ref = self._properties.get('jobReference') + if job_ref is not None: + self._job = QueryJob(job_ref['jobId'], self.query, + self._client) + return self._job @property def page_token(self): diff --git a/gcloud/bigquery/test_query.py b/gcloud/bigquery/test_query.py index b1d323e80bed..bed46d9e85e3 100644 --- a/gcloud/bigquery/test_query.py +++ b/gcloud/bigquery/test_query.py @@ -156,6 +156,8 @@ def test_job_w_jobid(self): self.assertEqual(job.query, self.QUERY) self.assertTrue(job._client is client) self.assertEqual(job.name, SERVER_GENERATED) + fetched_later = query.job + self.assertTrue(fetched_later is job) def test_schema(self): client = _Client(self.PROJECT) From e770e9e620f9d71cd76ea3883c175d581c855d83 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 29 Mar 2016 14:22:24 -0400 Subject: [PATCH 06/12] Rename 'readers'->'viewers', 'writers'->'editors'. Improve correspondence w/ role names used by pubsub. Addresses: https://github.com/GoogleCloudPlatform/gcloud-python/pull/1654#discussion_r57762370 https://github.com/GoogleCloudPlatform/gcloud-python/pull/1654#discussion_r57762391. --- gcloud/pubsub/iam.py | 16 +++++----- gcloud/pubsub/test_iam.py | 48 +++++++++++++++--------------- gcloud/pubsub/test_subscription.py | 48 +++++++++++++++--------------- gcloud/pubsub/test_topic.py | 48 +++++++++++++++--------------- 4 files changed, 80 insertions(+), 80 deletions(-) diff --git a/gcloud/pubsub/iam.py b/gcloud/pubsub/iam.py index 064acbb48682..5c9b2eeac603 100644 --- a/gcloud/pubsub/iam.py +++ b/gcloud/pubsub/iam.py @@ -40,8 +40,8 @@ def __init__(self, etag=None, version=None): self.etag = etag self.version = version self.owners = set() - self.writers = set() - self.readers = set() + self.editors = set() + self.viewers = set() @staticmethod def user(email): @@ -128,9 +128,9 @@ def from_api_repr(cls, resource): if role == OWNER_ROLE: policy.owners = members elif role == EDITOR_ROLE: - policy.writers = members + policy.editors = members elif role == VIEWER_ROLE: - policy.readers = members + policy.viewers = members else: raise ValueError('Unknown role: %s' % (role,)) return policy @@ -155,13 +155,13 @@ def to_api_repr(self): bindings.append( {'role': OWNER_ROLE, 'members': sorted(self.owners)}) - if self.writers: + if self.editors: bindings.append( - {'role': EDITOR_ROLE, 'members': sorted(self.writers)}) + {'role': EDITOR_ROLE, 'members': sorted(self.editors)}) - if self.readers: + if self.viewers: bindings.append( - {'role': VIEWER_ROLE, 'members': sorted(self.readers)}) + {'role': VIEWER_ROLE, 'members': sorted(self.viewers)}) if bindings: resource['bindings'] = bindings diff --git a/gcloud/pubsub/test_iam.py b/gcloud/pubsub/test_iam.py index dfd0087d8ba0..4aec6ad14130 100644 --- a/gcloud/pubsub/test_iam.py +++ b/gcloud/pubsub/test_iam.py @@ -29,8 +29,8 @@ def test_ctor_defaults(self): self.assertEqual(policy.etag, None) self.assertEqual(policy.version, None) self.assertEqual(list(policy.owners), []) - self.assertEqual(list(policy.writers), []) - self.assertEqual(list(policy.readers), []) + self.assertEqual(list(policy.editors), []) + self.assertEqual(list(policy.viewers), []) def test_ctor_explicit(self): VERSION = 17 @@ -39,8 +39,8 @@ def test_ctor_explicit(self): self.assertEqual(policy.etag, ETAG) self.assertEqual(policy.version, VERSION) self.assertEqual(list(policy.owners), []) - self.assertEqual(list(policy.writers), []) - self.assertEqual(list(policy.readers), []) + self.assertEqual(list(policy.editors), []) + self.assertEqual(list(policy.viewers), []) def test_user(self): EMAIL = 'phred@example.com' @@ -83,24 +83,24 @@ def test_from_api_repr_only_etag(self): self.assertEqual(policy.etag, 'ACAB') self.assertEqual(policy.version, None) self.assertEqual(list(policy.owners), []) - self.assertEqual(list(policy.writers), []) - self.assertEqual(list(policy.readers), []) + self.assertEqual(list(policy.editors), []) + self.assertEqual(list(policy.viewers), []) def test_from_api_repr_complete(self): from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE OWNER1 = 'user:phred@example.com' OWNER2 = 'group:cloud-logs@google.com' - WRITER1 = 'domain:google.com' - WRITER2 = 'user:phred@example.com' - READER1 = 'serviceAccount:1234-abcdef@service.example.com' - READER2 = 'user:phred@example.com' + EDITOR1 = 'domain:google.com' + EDITOR2 = 'user:phred@example.com' + VIEWER1 = 'serviceAccount:1234-abcdef@service.example.com' + VIEWER2 = 'user:phred@example.com' RESOURCE = { 'etag': 'DEADBEEF', 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [EDITOR1, EDITOR2]}, + {'role': VIEWER_ROLE, 'members': [VIEWER1, VIEWER2]}, ], } klass = self._getTargetClass() @@ -108,8 +108,8 @@ def test_from_api_repr_complete(self): self.assertEqual(policy.etag, 'DEADBEEF') self.assertEqual(policy.version, 17) self.assertEqual(sorted(policy.owners), [OWNER2, OWNER1]) - self.assertEqual(sorted(policy.writers), [WRITER1, WRITER2]) - self.assertEqual(sorted(policy.readers), [READER1, READER2]) + self.assertEqual(sorted(policy.editors), [EDITOR1, EDITOR2]) + self.assertEqual(sorted(policy.viewers), [VIEWER1, VIEWER2]) def test_from_api_repr_bad_role(self): BOGUS1 = 'user:phred@example.com' @@ -137,24 +137,24 @@ def test_to_api_repr_full(self): from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE OWNER1 = 'group:cloud-logs@google.com' OWNER2 = 'user:phred@example.com' - WRITER1 = 'domain:google.com' - WRITER2 = 'user:phred@example.com' - READER1 = 'serviceAccount:1234-abcdef@service.example.com' - READER2 = 'user:phred@example.com' + EDITOR1 = 'domain:google.com' + EDITOR2 = 'user:phred@example.com' + VIEWER1 = 'serviceAccount:1234-abcdef@service.example.com' + VIEWER2 = 'user:phred@example.com' EXPECTED = { 'etag': 'DEADBEEF', 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [EDITOR1, EDITOR2]}, + {'role': VIEWER_ROLE, 'members': [VIEWER1, VIEWER2]}, ], } policy = self._makeOne('DEADBEEF', 17) policy.owners.add(OWNER1) policy.owners.add(OWNER2) - policy.writers.add(WRITER1) - policy.writers.add(WRITER2) - policy.readers.add(READER1) - policy.readers.add(READER2) + policy.editors.add(EDITOR1) + policy.editors.add(EDITOR2) + policy.viewers.add(VIEWER1) + policy.viewers.add(VIEWER2) self.assertEqual(policy.to_api_repr(), EXPECTED) diff --git a/gcloud/pubsub/test_subscription.py b/gcloud/pubsub/test_subscription.py index de1ac8764e28..5672717c375b 100644 --- a/gcloud/pubsub/test_subscription.py +++ b/gcloud/pubsub/test_subscription.py @@ -488,17 +488,17 @@ def test_get_iam_policy_w_bound_client(self): from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE OWNER1 = 'user:phred@example.com' OWNER2 = 'group:cloud-logs@google.com' - WRITER1 = 'domain:google.com' - WRITER2 = 'user:phred@example.com' - READER1 = 'serviceAccount:1234-abcdef@service.example.com' - READER2 = 'user:phred@example.com' + EDITOR1 = 'domain:google.com' + EDITOR2 = 'user:phred@example.com' + VIEWER1 = 'serviceAccount:1234-abcdef@service.example.com' + VIEWER2 = 'user:phred@example.com' POLICY = { 'etag': 'DEADBEEF', 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [EDITOR1, EDITOR2]}, + {'role': VIEWER_ROLE, 'members': [VIEWER1, VIEWER2]}, ], } PROJECT = 'PROJECT' @@ -517,8 +517,8 @@ def test_get_iam_policy_w_bound_client(self): self.assertEqual(policy.etag, 'DEADBEEF') self.assertEqual(policy.version, 17) self.assertEqual(sorted(policy.owners), [OWNER2, OWNER1]) - self.assertEqual(sorted(policy.writers), [WRITER1, WRITER2]) - self.assertEqual(sorted(policy.readers), [READER1, READER2]) + self.assertEqual(sorted(policy.editors), [EDITOR1, EDITOR2]) + self.assertEqual(sorted(policy.viewers), [VIEWER1, VIEWER2]) self.assertEqual(len(conn._requested), 1) req = conn._requested[0] @@ -547,8 +547,8 @@ def test_get_iam_policy_w_alternate_client(self): self.assertEqual(policy.etag, 'ACAB') self.assertEqual(policy.version, None) self.assertEqual(sorted(policy.owners), []) - self.assertEqual(sorted(policy.writers), []) - self.assertEqual(sorted(policy.readers), []) + self.assertEqual(sorted(policy.editors), []) + self.assertEqual(sorted(policy.viewers), []) self.assertEqual(len(conn1._requested), 0) self.assertEqual(len(conn2._requested), 1) @@ -561,17 +561,17 @@ def test_set_iam_policy_w_bound_client(self): from gcloud.pubsub.iam import Policy OWNER1 = 'group:cloud-logs@google.com' OWNER2 = 'user:phred@example.com' - WRITER1 = 'domain:google.com' - WRITER2 = 'user:phred@example.com' - READER1 = 'serviceAccount:1234-abcdef@service.example.com' - READER2 = 'user:phred@example.com' + EDITOR1 = 'domain:google.com' + EDITOR2 = 'user:phred@example.com' + VIEWER1 = 'serviceAccount:1234-abcdef@service.example.com' + VIEWER2 = 'user:phred@example.com' POLICY = { 'etag': 'DEADBEEF', 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [EDITOR1, EDITOR2]}, + {'role': VIEWER_ROLE, 'members': [VIEWER1, VIEWER2]}, ], } RESPONSE = POLICY.copy() @@ -590,18 +590,18 @@ def test_set_iam_policy_w_bound_client(self): policy = Policy('DEADBEEF', 17) policy.owners.add(OWNER1) policy.owners.add(OWNER2) - policy.writers.add(WRITER1) - policy.writers.add(WRITER2) - policy.readers.add(READER1) - policy.readers.add(READER2) + policy.editors.add(EDITOR1) + policy.editors.add(EDITOR2) + policy.viewers.add(VIEWER1) + policy.viewers.add(VIEWER2) new_policy = subscription.set_iam_policy(policy) self.assertEqual(new_policy.etag, 'ABACABAF') self.assertEqual(new_policy.version, 18) self.assertEqual(sorted(new_policy.owners), [OWNER1, OWNER2]) - self.assertEqual(sorted(new_policy.writers), [WRITER1, WRITER2]) - self.assertEqual(sorted(new_policy.readers), [READER1, READER2]) + self.assertEqual(sorted(new_policy.editors), [EDITOR1, EDITOR2]) + self.assertEqual(sorted(new_policy.viewers), [VIEWER1, VIEWER2]) self.assertEqual(len(conn._requested), 1) req = conn._requested[0] @@ -631,8 +631,8 @@ def test_set_iam_policy_w_alternate_client(self): self.assertEqual(new_policy.etag, 'ACAB') self.assertEqual(new_policy.version, None) self.assertEqual(sorted(new_policy.owners), []) - self.assertEqual(sorted(new_policy.writers), []) - self.assertEqual(sorted(new_policy.readers), []) + self.assertEqual(sorted(new_policy.editors), []) + self.assertEqual(sorted(new_policy.viewers), []) self.assertEqual(len(conn1._requested), 0) self.assertEqual(len(conn2._requested), 1) diff --git a/gcloud/pubsub/test_topic.py b/gcloud/pubsub/test_topic.py index 96830c789bb6..c6967bfe4b72 100644 --- a/gcloud/pubsub/test_topic.py +++ b/gcloud/pubsub/test_topic.py @@ -456,17 +456,17 @@ def test_get_iam_policy_w_bound_client(self): from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE OWNER1 = 'user:phred@example.com' OWNER2 = 'group:cloud-logs@google.com' - WRITER1 = 'domain:google.com' - WRITER2 = 'user:phred@example.com' - READER1 = 'serviceAccount:1234-abcdef@service.example.com' - READER2 = 'user:phred@example.com' + EDITOR1 = 'domain:google.com' + EDITOR2 = 'user:phred@example.com' + VIEWER1 = 'serviceAccount:1234-abcdef@service.example.com' + VIEWER2 = 'user:phred@example.com' POLICY = { 'etag': 'DEADBEEF', 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [EDITOR1, EDITOR2]}, + {'role': VIEWER_ROLE, 'members': [VIEWER1, VIEWER2]}, ], } TOPIC_NAME = 'topic_name' @@ -483,8 +483,8 @@ def test_get_iam_policy_w_bound_client(self): self.assertEqual(policy.etag, 'DEADBEEF') self.assertEqual(policy.version, 17) self.assertEqual(sorted(policy.owners), [OWNER2, OWNER1]) - self.assertEqual(sorted(policy.writers), [WRITER1, WRITER2]) - self.assertEqual(sorted(policy.readers), [READER1, READER2]) + self.assertEqual(sorted(policy.editors), [EDITOR1, EDITOR2]) + self.assertEqual(sorted(policy.viewers), [VIEWER1, VIEWER2]) self.assertEqual(len(conn._requested), 1) req = conn._requested[0] @@ -511,8 +511,8 @@ def test_get_iam_policy_w_alternate_client(self): self.assertEqual(policy.etag, 'ACAB') self.assertEqual(policy.version, None) self.assertEqual(sorted(policy.owners), []) - self.assertEqual(sorted(policy.writers), []) - self.assertEqual(sorted(policy.readers), []) + self.assertEqual(sorted(policy.editors), []) + self.assertEqual(sorted(policy.viewers), []) self.assertEqual(len(conn1._requested), 0) self.assertEqual(len(conn2._requested), 1) @@ -525,17 +525,17 @@ def test_set_iam_policy_w_bound_client(self): from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE OWNER1 = 'group:cloud-logs@google.com' OWNER2 = 'user:phred@example.com' - WRITER1 = 'domain:google.com' - WRITER2 = 'user:phred@example.com' - READER1 = 'serviceAccount:1234-abcdef@service.example.com' - READER2 = 'user:phred@example.com' + EDITOR1 = 'domain:google.com' + EDITOR2 = 'user:phred@example.com' + VIEWER1 = 'serviceAccount:1234-abcdef@service.example.com' + VIEWER2 = 'user:phred@example.com' POLICY = { 'etag': 'DEADBEEF', 'version': 17, 'bindings': [ {'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]}, - {'role': EDITOR_ROLE, 'members': [WRITER1, WRITER2]}, - {'role': VIEWER_ROLE, 'members': [READER1, READER2]}, + {'role': EDITOR_ROLE, 'members': [EDITOR1, EDITOR2]}, + {'role': VIEWER_ROLE, 'members': [VIEWER1, VIEWER2]}, ], } RESPONSE = POLICY.copy() @@ -552,18 +552,18 @@ def test_set_iam_policy_w_bound_client(self): policy = Policy('DEADBEEF', 17) policy.owners.add(OWNER1) policy.owners.add(OWNER2) - policy.writers.add(WRITER1) - policy.writers.add(WRITER2) - policy.readers.add(READER1) - policy.readers.add(READER2) + policy.editors.add(EDITOR1) + policy.editors.add(EDITOR2) + policy.viewers.add(VIEWER1) + policy.viewers.add(VIEWER2) new_policy = topic.set_iam_policy(policy) self.assertEqual(new_policy.etag, 'ABACABAF') self.assertEqual(new_policy.version, 18) self.assertEqual(sorted(new_policy.owners), [OWNER1, OWNER2]) - self.assertEqual(sorted(new_policy.writers), [WRITER1, WRITER2]) - self.assertEqual(sorted(new_policy.readers), [READER1, READER2]) + self.assertEqual(sorted(new_policy.editors), [EDITOR1, EDITOR2]) + self.assertEqual(sorted(new_policy.viewers), [VIEWER1, VIEWER2]) self.assertEqual(len(conn._requested), 1) req = conn._requested[0] @@ -591,8 +591,8 @@ def test_set_iam_policy_w_alternate_client(self): self.assertEqual(new_policy.etag, 'ACAB') self.assertEqual(new_policy.version, None) self.assertEqual(sorted(new_policy.owners), []) - self.assertEqual(sorted(new_policy.writers), []) - self.assertEqual(sorted(new_policy.readers), []) + self.assertEqual(sorted(new_policy.editors), []) + self.assertEqual(sorted(new_policy.viewers), []) self.assertEqual(len(conn1._requested), 0) self.assertEqual(len(conn2._requested), 1) From da06d10477a139e61b3fefc4d8c3569fec5670f7 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 29 Mar 2016 14:25:06 -0400 Subject: [PATCH 07/12] Use role-name constants. Addresses: https://github.com/GoogleCloudPlatform/gcloud-python/pull/1654#discussion_r57762554 https://github.com/GoogleCloudPlatform/gcloud-python/pull/1654#discussion_r57762583 --- gcloud/pubsub/test_subscription.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gcloud/pubsub/test_subscription.py b/gcloud/pubsub/test_subscription.py index 5672717c375b..1b8ea750643a 100644 --- a/gcloud/pubsub/test_subscription.py +++ b/gcloud/pubsub/test_subscription.py @@ -642,12 +642,13 @@ def test_set_iam_policy_w_alternate_client(self): self.assertEqual(req['data'], {'policy': {}}) def test_check_iam_permissions_w_bound_client(self): + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE PROJECT = 'PROJECT' TOPIC_NAME = 'topic_name' SUB_NAME = 'sub_name' PATH = 'projects/%s/subscriptions/%s:testIamPermissions' % ( PROJECT, SUB_NAME) - ROLES = ['roles/viewer', 'roles/editor', 'roles/owner'] + ROLES = [VIEWER_ROLE, EDITOR_ROLE, OWNER_ROLE] REQUESTED = { 'permissions': ROLES, } @@ -669,12 +670,13 @@ def test_check_iam_permissions_w_bound_client(self): self.assertEqual(req['data'], REQUESTED) def test_check_iam_permissions_w_alternate_client(self): + from gcloud.pubsub.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE PROJECT = 'PROJECT' TOPIC_NAME = 'topic_name' SUB_NAME = 'sub_name' PATH = 'projects/%s/subscriptions/%s:testIamPermissions' % ( PROJECT, SUB_NAME) - ROLES = ['roles/viewer', 'roles/editor', 'roles/owner'] + ROLES = [VIEWER_ROLE, EDITOR_ROLE, OWNER_ROLE] REQUESTED = { 'permissions': ROLES, } From 0116b18dc19c9c6a192bedfbf7223c20244264d0 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 29 Mar 2016 14:26:51 -0400 Subject: [PATCH 08/12] Assert created topic/subscription exists before use. Addresses: https://github.com/GoogleCloudPlatform/gcloud-python/pull/1654#discussion_r57762833 https://github.com/GoogleCloudPlatform/gcloud-python/pull/1654#discussion_r57763265. --- system_tests/pubsub.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/system_tests/pubsub.py b/system_tests/pubsub.py index cce3e0118d04..813be010dc78 100644 --- a/system_tests/pubsub.py +++ b/system_tests/pubsub.py @@ -177,6 +177,7 @@ def test_topic_iam_policy(self): while count > 0 and not topic.exists(): time.sleep(1) count -= 1 + self.assertTrue(topic.exists()) self.to_delete.append(topic) policy = topic.get_iam_policy() policy.readers.add(policy.user('jjg@google.com')) @@ -200,6 +201,7 @@ def test_subscription_iam_policy(self): while count > 0 and not subscription.exists(): time.sleep(1) count -= 1 + self.assertTrue(subscription.exists()) self.to_delete.insert(0, subscription) policy = subscription.get_iam_policy() policy.readers.add(policy.user('jjg@google.com')) From 9ed69971bf200eb95c4c961dc6643f211d7485ce Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 29 Mar 2016 13:48:31 -0400 Subject: [PATCH 09/12] Expand/clarify synchronous query usage docs. Address three separate cases: - Query finishes within timeout, with all rows present. - Query finishes within timeout, but result set is too large for initial response. - Query times out. Closes #1551. --- docs/bigquery-usage.rst | 88 +++++++++++++++++++++++++++++++++++------ 1 file changed, 75 insertions(+), 13 deletions(-) diff --git a/docs/bigquery-usage.rst b/docs/bigquery-usage.rst index f84bf638f3cb..edab2e1510d4 100644 --- a/docs/bigquery-usage.rst +++ b/docs/bigquery-usage.rst @@ -291,26 +291,88 @@ Run a query which can be expected to complete within bounded time: >>> from gcloud import bigquery >>> client = bigquery.Client() - >>> query = """\ - SELECT count(*) AS age_count FROM dataset_name.person_ages - """ - >>> query = client.run_sync_query(query) + >>> QUERY = """\ + ... SELECT count(*) AS age_count FROM dataset_name.person_ages + ... """ + >>> query = client.run_sync_query(QUERY) + >>> query.timeout_ms = 1000 + >>> query.run() # API request + >>> query.complete + True + >>> len(query.schema) + 1 + >>> field = query.schema[0] + >>> field.name + u'count' + >>> field.field_type + u'INTEGER' + >>> field.mode + u'NULLABLE' + >>> query.rows + [(15,)] + >>> query.total_rows + 1 + +If the rows returned by the query do not fit into the inital response, +then we need to fetch the remaining rows via ``fetch_data``: + +.. doctest:: + + >>> from gcloud import bigquery + >>> client = bigquery.Client() + >>> QUERY = """\ + ... SELECT * FROM dataset_name.person_ages + ... """ + >>> query = client.run_sync_query(QUERY) + >>> query.timeout_ms = 1000 + >>> query.run() # API request + >>> query.complete + True + >>> query.total_rows + 1234 + >>> query.page_token + '8d6e452459238eb0fe87d8eb191dd526ee70a35e' + >>> do_something_with(query.schema, query.rows) + >>> token = query.page_token # for initial request + >>> while True: + ... do_something_with(query.schema, rows) + ... if token is None: + ... break + ... rows, _, token = query.fetch_data(page_token=token) + + +If the query takes longer than the timeout allowed, ``query.complete`` +will be ``False``. In that case, we need to poll the associated job until +it is done, and then fetch the reuslts: + +.. doctest:: + + >>> from gcloud import bigquery + >>> client = bigquery.Client() + >>> QUERY = """\ + ... SELECT * FROM dataset_name.person_ages + ... """ + >>> query = client.run_sync_query(QUERY) >>> query.timeout_ms = 1000 >>> query.run() # API request + >>> query.complete + False + >>> job = query.job >>> retry_count = 100 - >>> while retry_count > 0 and not job.complete: + >>> while retry_count > 0 and job.state == 'running': ... retry_count -= 1 ... time.sleep(10) - ... query.reload() # API request - >>> query.schema - [{'name': 'age_count', 'type': 'integer', 'mode': 'nullable'}] - >>> query.rows - [(15,)] + ... job.reload() # API call + >>> job.state + 'done' + >>> token = None # for initial request + >>> while True: + ... rows, _, token = query.fetch_data(page_token=token) + ... do_something_with(query.schema, rows) + ... if token is None: + ... break -.. note:: - If the query takes longer than the timeout allowed, ``job.complete`` - will be ``False``: we therefore poll until it is completed. Querying data (asynchronous) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From 0afb7b55a08ce1189c4aef4215156afefeae5912 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 23 Mar 2016 11:57:59 -0400 Subject: [PATCH 10/12] Add system test for sink creation w/ pubsub topic. Broken because 'Topic.set_iam_policy' returns 503s. See: #1654. --- system_tests/logging_.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/system_tests/logging_.py b/system_tests/logging_.py index fdef82679479..46ed1f8bb26c 100644 --- a/system_tests/logging_.py +++ b/system_tests/logging_.py @@ -29,6 +29,7 @@ DEFAULT_DESCRIPTION = 'System testing' BUCKET_NAME = 'gcloud-python-system-testing-%d' % (_MILLIS,) DATASET_NAME = 'system_testing_dataset_%d' % (_MILLIS,) +TOPIC_NAME = 'gcloud-python-system-testing-%d' % (_MILLIS,) class Config(object): @@ -51,8 +52,12 @@ def setUp(self): self.to_delete = [] def tearDown(self): + from gcloud.exceptions import NotFound for doomed in self.to_delete: - doomed.delete() + try: + doomed.delete() + except NotFound as error: + print('Unable to delete resource: %s' % error) def test_log_text(self): TEXT_PAYLOAD = 'System test: test_log_text' @@ -175,3 +180,25 @@ def test_create_sink_bigquery_dataset(self): sink.create() self.to_delete.append(sink) self.assertTrue(sink.exists()) + + def test_create_sink_pubsub_topic(self): + from gcloud import pubsub + TOPIC_URI = 'pubsub.googleapis.com/projects/%s/topic/%s' % ( + Config.CLIENT.project, TOPIC_NAME) + + # Create the destination topic, and set up the IAM policy to allow + # Cloud Logging to write into it. + pubsub_client = pubsub.Client() + topic = pubsub_client.topic(TOPIC_NAME) + topic.create() + self.to_delete.append(topic) + policy = topic.get_iam_policy() + policy.owners.add(policy.group('cloud-logs@google.com')) + topic.set_iam_policy(policy) + + sink = Config.CLIENT.sink( + DEFAULT_SINK_NAME, DEFAULT_FILTER, TOPIC_URI) + self.assertFalse(sink.exists()) + sink.create() + self.to_delete.append(sink) + self.assertTrue(sink.exists()) From fb0f813355a41a48ed096d0c88813a34261c8fcc Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 30 Mar 2016 12:16:11 -0400 Subject: [PATCH 11/12] Back out swallowing/loggin teardown failures due to #1657. Addresses: https://github.com/GoogleCloudPlatform/gcloud-python/pull/1656#discussion_r57595585 https://github.com/GoogleCloudPlatform/gcloud-python/pull/1656#discussion_r57917947 --- system_tests/logging_.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/system_tests/logging_.py b/system_tests/logging_.py index 46ed1f8bb26c..32c4d00ba6a1 100644 --- a/system_tests/logging_.py +++ b/system_tests/logging_.py @@ -52,12 +52,8 @@ def setUp(self): self.to_delete = [] def tearDown(self): - from gcloud.exceptions import NotFound for doomed in self.to_delete: - try: - doomed.delete() - except NotFound as error: - print('Unable to delete resource: %s' % error) + doomed.delete() def test_log_text(self): TEXT_PAYLOAD = 'System test: test_log_text' From 8ca22dc26c9282bdc9f8454d3950bfeb15708fe4 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 30 Mar 2016 12:58:47 -0400 Subject: [PATCH 12/12] Use 'topic.full_name' to compute destination URI. Addresses: https://github.com/GoogleCloudPlatform/gcloud-python/pull/1656#discussion_r57919574 --- system_tests/logging_.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/system_tests/logging_.py b/system_tests/logging_.py index 32c4d00ba6a1..6d3440494fe2 100644 --- a/system_tests/logging_.py +++ b/system_tests/logging_.py @@ -179,8 +179,6 @@ def test_create_sink_bigquery_dataset(self): def test_create_sink_pubsub_topic(self): from gcloud import pubsub - TOPIC_URI = 'pubsub.googleapis.com/projects/%s/topic/%s' % ( - Config.CLIENT.project, TOPIC_NAME) # Create the destination topic, and set up the IAM policy to allow # Cloud Logging to write into it. @@ -192,6 +190,8 @@ def test_create_sink_pubsub_topic(self): policy.owners.add(policy.group('cloud-logs@google.com')) topic.set_iam_policy(policy) + TOPIC_URI = 'pubsub.googleapis.com/%s' % (topic.full_name,) + sink = Config.CLIENT.sink( DEFAULT_SINK_NAME, DEFAULT_FILTER, TOPIC_URI) self.assertFalse(sink.exists())