Skip to content
This repository was archived by the owner on Nov 12, 2025. It is now read-only.

test: use job metadata to avoid test flakes#155

Merged
tswast merged 3 commits intogoogleapis:masterfrom
tswast:issue151-snapshot-flakes
Mar 16, 2021
Merged

test: use job metadata to avoid test flakes#155
tswast merged 3 commits intogoogleapis:masterfrom
tswast:issue151-snapshot-flakes

Conversation

@tswast
Copy link
Copy Markdown
Contributor

@tswast tswast commented Mar 15, 2021

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #151 🦕

@tswast tswast requested review from a team and shollyman and removed request for a team March 15, 2021 19:52
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Mar 15, 2021
@product-auto-label product-auto-label bot added the api: bigquerystorage Issues related to the googleapis/python-bigquery-storage API. label Mar 15, 2021
@tswast tswast requested a review from crwilcox March 15, 2021 20:06
@tswast
Copy link
Copy Markdown
Contributor Author

tswast commented Mar 15, 2021

@crwilcox I am trying with a sleep since I suspect clock sync issues. Would you prefer I use pytest-repeat / flaky as you suggest in go/handling-flakes-examples?

@crwilcox
Copy link
Copy Markdown
Contributor

@tswast I think this is reasonable. If you see the go example this is the strategy there, though we don't wait 10s. Instead it does an exponential backoff up to 3 times. That way most cases don't take that much longer.

# Sleep for a moment to give use some wiggle room in case the BigQuery
# snapshot time and our times are out-of-sync.
# https://github.com/googleapis/python-bigquery-storage/issues/151
time.sleep(10)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the point of the initial sleep? it seems the second is to ensure that the snapshot time requested is in the past?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The flake is actually that there are 0 rows, so the snapshot time is too early in that case.

That said, Seth makes a good point that I can use the times from the table metadata.

Copy link
Copy Markdown
Contributor

@shollyman shollyman left a comment

Choose a reason for hiding this comment

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

If you have a reference to the table metadata as well as the job statistics from the load you're doing, you may be able to adapt the logic from the table delete/undelete snippet and avoid the wasted time sleeping.

@tswast tswast changed the title test: add sleep to avoid test flakes test: use job metadata to avoid test flakes Mar 16, 2021
@tswast tswast requested a review from shollyman March 16, 2021 14:48
@tswast
Copy link
Copy Markdown
Contributor Author

tswast commented Mar 16, 2021

@shollyman Good point! I've updated the test to use the times from the job metadata.

@tswast tswast merged commit 8022e9d into googleapis:master Mar 16, 2021
@tswast tswast deleted the issue151-snapshot-flakes branch March 16, 2021 17:48
@tswast
Copy link
Copy Markdown
Contributor Author

tswast commented Mar 17, 2021

This test flaked again. https://source.cloud.google.com/results/invocations/f821c579-a7fb-45a7-9110-dad4dd830842/targets/github%2Fpython-bigquery-storage/tests

Looks like we should add a _retry_403 like we have in other BQ tests to deal with rate limiting.

gcf-merge-on-green bot pushed a commit that referenced this pull request Mar 17, 2021
Follow-up to #155

With 2 load jobs right in a row, we see to be hitting rate limiting. Add `retry_403` from `google-cloud-bigquery` tests, which seem to successfully work around this.

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
- [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/python-bigquery-storage/issues/new/choose) before writing your code!  That way we can discuss the change, evaluate designs, and agree on the general idea
- [ ] Ensure the tests and linter pass
- [ ] Code coverage does not decrease (if any source code was changed)
- [ ] Appropriate docs were updated (if necessary)

Fixes #151
Fixes #161  🦕
parthea pushed a commit to googleapis/google-cloud-python that referenced this pull request Aug 21, 2025
Follow-up to googleapis/python-bigquery-storage#155

With 2 load jobs right in a row, we see to be hitting rate limiting. Add `retry_403` from `google-cloud-bigquery` tests, which seem to successfully work around this.

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
- [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/python-bigquery-storage/issues/new/choose) before writing your code!  That way we can discuss the change, evaluate designs, and agree on the general idea
- [ ] Ensure the tests and linter pass
- [ ] Code coverage does not decrease (if any source code was changed)
- [ ] Appropriate docs were updated (if necessary)

Fixes #151
Fixes #161  🦕
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: bigquerystorage Issues related to the googleapis/python-bigquery-storage API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tests.system.reader.test_reader: test_snapshot[v1] failed

3 participants