Skip to content

Conversation

@tmatsuo
Copy link
Contributor

@tmatsuo tmatsuo commented Apr 9, 2020

fixes #3316

followup on #3278

@tmatsuo tmatsuo requested review from gguuss and leahecole April 9, 2020 00:39
@tmatsuo tmatsuo requested a review from a team as a code owner April 9, 2020 00:39
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 9, 2020

def test_auto_complete_sample(company_name, capsys):
@eventually_consistent.call
@eventually_consistent.call(tries=5)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we switch to flaky or retry over eventually_consistent ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kurtisvg Thanks for the comment. To be honest, we have different opinions.

I think eventually_consistent has some benefit. It gives semantics.

It marks the tests as eventually consistent, which means it is a correct thing to retry.
I quite don't like to use flaky or retry for eventually consistent tests, because we can not distinguish eventually consistent tests from usual flakes.

Also, it is a great shortcut for eventually consistent issues with exponential back off. There's no need for defining helper callback.

So, for eventually consistent tests, I strongly want to stick with eventually_consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, if we ultimately decide to eliminate gcp-devrel-py-tools, I'll replace eventually_consistent with retrying. However, I'll stick to eventally_consistent in this PR.

Can you file an issue for eliminating gcp-devrel-py-tools so that we can discuss further there?

@tmatsuo
Copy link
Contributor Author

tmatsuo commented Apr 9, 2020

The error:

assert re.search('COMPANY_ID', out)

AssertionError: assert None

  • where None = <function search at 0x7fce75338e18>('COMPANY_ID', "Retrying due to eventual consistency.\n{'metadata': {'requestId': '880e787a-99f3-48ea-8d79-a2623e95d1be:APAb7IR0Cpj1XDbIgvr/m71HC7iIXvI+nQ=='}}\n")

It seems like the test is failing correctly? The API response seems different from usual one.

@tmatsuo
Copy link
Contributor Author

tmatsuo commented Apr 9, 2020

Filed: #3318

It passes 100 times locally. I'd just re-run the test for now.

@tmatsuo
Copy link
Contributor Author

tmatsuo commented Apr 10, 2020

We decided to remove gcp-devrel-py-tools from the repo, closing.

@tmatsuo tmatsuo closed this Apr 10, 2020
@tmatsuo tmatsuo deleted the fix-jobs branch April 22, 2020 23:57
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.

[jobs] testing: add tries argument on @eventually_consistent.call decorator

3 participants