Add system tests for topic/subscription IAM policy get/set methods. by tseaver · Pull Request #1654 · googleapis/google-cloud-python · GitHub

The tests uncover a wart in the API: setIamPermissions takes an extra wrapper element (policy) around the actual Policy resource. I don't know where to report that issue.

The new system tests fail repeatedly for my system account with 503s: adding retries (interspersed with time.sleep(1)) doesn't seem to help. @tmatsuo can you comment? (Note that I have made that account an owner of my project).

Mostly looks fine though we should resolve the 503s before I gave an LGTM. Assigned to @tmatsuo for now.

@tmatsuo Bump

Which test and API are repeatedly failing? Any logs? Detailed message? Does it always fail or sometimes succeed? Can you show the retry code?

@tmatsuo the new system tests being added in this PR fail with 503s at the following points.

Wrapping the set_iam_policy calls with retry logic didn't help, so I removed it.

Does it always fail? If so, is it possible to show the actual JSON request?

@tmatsuo Just before the projects.topics.setIamPolicy API call:

(Pdb) l
298             """
299             client = self._require_client(client)
300             path = '%s:setIamPolicy' % (self.path,)
301             resource = policy.to_api_repr()
302             wrapped = {'policy': resource}
303  ->         resp = client.connection.api_request(
304                 method='POST', path=path, data=wrapped)
305             return Policy.from_api_repr(resp)
(pdb) pp path
u'/projects/citric-celerity-697/topics/test-iam-policy-topic1459191212607:setIamPolicy'
(Pdb) pp wrapped
{'policy': {'bindings': [{'members': ['user:jgeewax@google.com'],
                          'role': 'roles/reader'}],
            'etag': u'ACAB'}}

At the return:

(Pdb) pp response
{'-content-encoding': 'gzip',
 'alt-svc': 'quic=":443"; ma=2592000; v="31,30,29,28,27,26,25"',
 'alternate-protocol': '443:quic,p=1',
 'cache-control': 'private',
 'content-length': '162',
 'content-type': 'application/json; charset=UTF-8',
 'date': 'Mon, 28 Mar 2016 18:55:54 GMT',
 'server': 'ESF',
 'status': '503',
 'transfer-encoding': 'chunked',
 'vary': 'Origin, X-Origin, Referer',
 'x-content-type-options': 'nosniff',
 'x-frame-options': 'SAMEORIGIN',
 'x-xss-protection': '1; mode=block'}
(Pdb) pp content
'{\n  "error": {\n    "code": 503,\n    "message": "The service was unable to fulfill your request. Please try again. [code=8a75]",\n    "status": "UNAVAILABLE"\n  }\n}\n'
(Pdb) pp url
'https://pubsub.googleapis.com/v1/projects/citric-celerity-697/topics/test-iam-policy-topic1459191212607:setIamPolicy'

@tmatsuo the projects.subscriptions.getIamPolicy call is now repeatable returning a 404 (waiting is not helping):

gcloud.exceptions.NotFound: 404 Resource not found (resource=test-iam-policy-sub-1459192208068). (GET https://pubsub.googleapis.com/v1/projects/citric-celerity-697/subscriptions/test-iam-policy-sub-1459192208068:getIamPolicy)

According to https://cloud.google.com/pubsub/access_control

I don't think roles/reader is a valid role. Can you try it with roles/viewer or roles/pubsub.viewer ?
The error message is not helpful at all, which should be improved though.

@tmatsuo After updating the role strings, I'm still getting the 503 from projects.topics.setIamPolicy. I have checked that the payload for the request now has the correct role:

-> resp = client.connection.api_request(
(Pdb) pp wrapped
{'policy': {'bindings': [{'members': ['user:jgeewax@google.com'],
                          'role': 'roles/viewer'}],
            'etag': u'ACAB'}}

FWIW: A 503 is a terrible status code for "you gave me bad data"; that status is supposed to mean "my backend went away unexpectedly, try again later." A better status would be a 40x (probably just 400 "Bad Request").

@tseaver

Indeed, the HTTP status code is terrible. I think the product team is working on it.

Maybe I found out the cause. I suspect the account jgeewax@google.com doesn't exist. Can you try using an existing account like tmatsuo@google.com?

@tmatsuo Indeed, fixing the e-mail address makes the setIamPolicy calls succeed. Now I just need to figure out why the teardown code blows up for the subscription case.

@dhermes the two new system tests now pass for me. PTAL

@tmatsuo thanks for the help!

Improve correspondence w/ role names used by pubsub. Addresses: #1654 (comment) #1654 (comment).

Addresses: #1654 (comment) #1654 (comment)

Addresses: #1654 (comment) #1654 (comment).

LGTM

* chore(deps): update all dependencies * revert * allow newer versions of libcst --------- Co-authored-by: Anthonios Partheniou <partheniou@google.com>

…upload (#1654) feat: send entire object checksum in the final api call of resumable upload fixes b/461994245 --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

🤖 I have created a release *beep* *boop* --- ## [3.7.0](googleapis/python-storage@v3.6.0...v3.7.0) (2025-12-09) ### Features * Auto enable mTLS when supported certificates are detected ([#1637](googleapis/python-storage#1637)) ([4e91c54](googleapis/python-storage@4e91c54)) * Send entire object checksum in the final api call of resumable upload ([#1654](googleapis/python-storage#1654)) ([ddce7e5](googleapis/python-storage@ddce7e5)) * Support urllib3 >= 2.6.0 ([#1658](googleapis/python-storage#1658)) ([57405e9](googleapis/python-storage@57405e9)) ### Bug Fixes * **bucket:** Move blob fails when the new blob name contains characters that need to be url encoded ([#1605](googleapis/python-storage#1605)) ([ec470a2](googleapis/python-storage@ec470a2)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Chandra Shekhar Sirimala <chandrasiri@google.com>

tseaver added testing api: pubsub Issues related to the Pub/Sub API. labels Mar 23, 2016

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 23, 2016

tseaver assigned dhermes Mar 23, 2016

dhermes reviewed Mar 24, 2016
View reviewed changes

dhermes assigned tmatsuo and unassigned dhermes Mar 24, 2016

tseaver mentioned this pull request Mar 24, 2016

Add system test for 'Sink.create' using a Pubsub topic. #1656

Closed

Add system tests for topic/subscription IAM policy get/set methods.

21cfae6

tseaver assigned dhermes and unassigned tmatsuo Mar 29, 2016

dhermes reviewed Mar 29, 2016
View reviewed changes

tseaver added 3 commits March 29, 2016 14:22

Rename 'readers'->'viewers', 'writers'->'editors'.

e770e9e

Improve correspondence w/ role names used by pubsub. Addresses: #1654 (comment) #1654 (comment).

Use role-name constants.

da06d10

Addresses: #1654 (comment) #1654 (comment)

Assert created topic/subscription exists before use.

0116b18

Addresses: #1654 (comment) #1654 (comment).

tseaver merged commit 7aafecd into googleapis:master Mar 29, 2016

tseaver deleted the pubsub-system_tests-iam_policy branch March 29, 2016 18:57

dhermes mentioned this pull request Apr 1, 2016

Upgrading version to 0.12.0 #1689

Merged

theacodes unassigned dhermes Sep 28, 2018

parthea added a commit that referenced this pull request Nov 24, 2025

chore(deps): update all dependencies (#1654 )

a5e5dbc

* chore(deps): update all dependencies * revert * allow newer versions of libcst --------- Co-authored-by: Anthonios Partheniou <partheniou@google.com>

parthea pushed a commit that referenced this pull request Apr 1, 2026

perf: Use flyweight for node fields (#1654 )

37bfbba

                               policy.owners = members
-                          elif role == WRITER_ROLE:
+                          elif role == EDITOR_ROLE:
                               policy.writers = members

+                      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)

Add system tests for topic/subscription IAM policy get/set methods. by tseaver · Pull Request #1654 · googleapis/google-cloud-python · GitHub

Uh oh!

Add system tests for topic/subscription IAM policy get/set methods. - #1654

Add system tests for topic/subscription IAM policy get/set methods.#1654tseaver merged 4 commits intogoogleapis:mastergoogleapis/google-cloud-python:masterfrom tseaver:pubsub-system_tests-iam_policyCopy head branch name to clipboard

Conversation

tseaver commented Mar 23, 2016

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

dhermes commented Mar 24, 2016

Uh oh!

dhermes commented Mar 28, 2016

Uh oh!

tmatsuo commented Mar 28, 2016

Uh oh!

tseaver commented Mar 28, 2016

Uh oh!

tmatsuo commented Mar 28, 2016

Uh oh!

tseaver commented Mar 28, 2016

Uh oh!

tseaver commented Mar 28, 2016

Uh oh!

tmatsuo commented Mar 28, 2016

Uh oh!

tseaver commented Mar 28, 2016

Uh oh!

tmatsuo commented Mar 29, 2016

Uh oh!

tseaver commented Mar 29, 2016

Uh oh!

tseaver commented Mar 29, 2016

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

dhermes commented Mar 29, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Add system tests for topic/subscription IAM policy get/set methods.#1654
tseaver merged 4 commits into
googleapis:masterfrom
tseaver:pubsub-system_tests-iam_policy