Skip to content

Conversation

daniel-sanche
Copy link
Member

fixes #2970

I added exponential backoff to the policy test, as recommended by the error message

@daniel-sanche daniel-sanche requested a review from a team as a code owner April 2, 2020 23:58
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 2, 2020
@daniel-sanche daniel-sanche requested a review from leahecole April 2, 2020 23:59
self.symId)
except Aborted:
# aborted by backend. Try again
try_number += 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you want to use @eventually_consistent.call?

An example commit:
c6254e4

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to using eventually consistent if you can

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know about @eventually_consistent, that's much cleaner!

self.symId)
except Aborted:
# aborted by backend. Try again
try_number += 1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to using eventually consistent if you can

if b.role == self.role and self.member in b.members:
found = True
assert found
eventually_consistent.call(check_policy,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately the decorator form doesn't work with the current 0.0.15 release.
See: GoogleCloudPlatform/python-repo-tools#25

I'm trying to make a new release of the above module.

I'm fine with the current form :)

Copy link
Contributor

@gguuss gguuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just some minor nits.

Copy link
Contributor

@tmatsuo tmatsuo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see my comment about exceptions tuple.

Copy link
Contributor

@tmatsuo tmatsuo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Ship it!

@tmatsuo
Copy link
Contributor

tmatsuo commented Apr 7, 2020

@dansanche Sorry for "hot" repo issue ;) I needed to rebase multiple times, but it's good to see the tests are more stable :)

@daniel-sanche daniel-sanche merged commit 5b5599f into master Apr 7, 2020
busunkim96 pushed a commit to googleapis/python-kms that referenced this pull request Jun 4, 2020
@anguillanneuf anguillanneuf deleted the kms-flaky-fix branch November 3, 2021 21:01
rsamborski pushed a commit that referenced this pull request Nov 8, 2022
rsamborski pushed a commit that referenced this pull request Nov 11, 2022
rsamborski pushed a commit that referenced this pull request Nov 14, 2022
dandhlee pushed a commit that referenced this pull request Nov 14, 2022
parthea pushed a commit to googleapis/google-cloud-python that referenced this pull request Oct 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kms.api-client.snippets_test.TestKMSSnippets: test_key_policy failed
5 participants