| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
This is not just "taking the easy way out", it's really the most appropriate fix since there are so many assertions that can fail due to eventual consistency. (For example, asking for 2 blobs should have a next page when 3 are in the bucket, but this may break down due to eventual consistency.) Fixes googleapis#2217. Also - restoring test_second_level() to pre-googleapis#2252 (by retrying the entire test case) - restoring test_list_files() to pre-googleapis#2181 (by retrying the entire test case) - adding retries around remaining test cases that call list_blobs(): test_root_level_w_delimiter(), test_first_level() and test_third_level() - adding a helper to empty a bucket in setUpClass() helper (which also uses list_blobs()) in both TestStoragePseudoHierarchy and TestStorageListFiles
|
@dhermes, one thing I thought about the issue you mentioned with RetryErrors, is that the exception can be passed in if you want a different exception caught instead of AssertionError. I also think you could stack RetryErrors decorators to capture more than one specific exception if I'm not mistaken. |
Sorry, something went wrong.
|
@daspecster Thanks for the tip, though I was aware of that behavior already. The point of this is that the failures are quite complex and to really change them to be eventual-consistency-proof would be equivalent to decorating the entire method, so we might as well. |
Sorry, something went wrong.
|
I'm doubtful that wrapping the entire testcase is going to make us more robust than wrapping the individual list_blobs calls: for testcase methods that call list_blobs more than once, we might need at the minimum to up the retry count, for instance. |
Sorry, something went wrong.
|
@tseaver None of the wrapped tests call list_blobs(), they fetch a single page and check information in the iterator as well. The tests need to check the state of so many different things, it's equivalent to just wrapping the entire function in an inner function and changing some of the asserts to part of the boolean checked by RetryResult. |
Sorry, something went wrong.
|
@tseaver Bump. This showed up again in: https://travis-ci.org/GoogleCloudPlatform/google-cloud-python/builds/160041019 |
Sorry, something went wrong.
Hmm? The all call list_blobs: I guess maybe you meant "none call it more than once?" I'm still dubious that we should assume any assertion failure in one of these tests is due to an eventual consistency error. |
Sorry, something went wrong.
|
@tseaver Regarding your doubt:
test_second_level and test_third_level are essentially the same as test_first_level |
Sorry, something went wrong.
|
I'll acquiesce, but still think we'd be better off repeating the list_blobs API request explicitly until it returned the expected number of files. LGTM |
Sorry, something went wrong.
Sorry, something went wrong.
…2293) Also: - include link to `bigframes.bigquery.ai` in README - add partial ordering mode recommendation to starter sample - remove 2.0 warning 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-dataframes/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) Towards b/454350869 🦕
PR created by the Librarian CLI to initialize a release. Merging this PR will auto trigger a release. Librarian Version: v0.7.0 Language Image: us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/python-librarian-generator@sha256:c8612d3fffb3f6a32353b2d1abd16b61e87811866f7ec9d65b59b02eb452a620 <details><summary>bigframes: 2.30.0</summary> ## [2.30.0](google/bigframes@v2.29.0...v2.30.0) (2025-12-03) ### Features * Support mixed scalar-analytic expressions (#2239) ([20ab469d](google/bigframes@20ab469d)) * Allow drop_duplicates over unordered dataframe (#2303) ([52665fa5](google/bigframes@52665fa5)) * Preserve source names better for more readable sql (#2243) ([64995d65](google/bigframes@64995d65)) * use end user credentials for `bigframes.bigquery.ai` functions when `connection_id` is not present (#2272) ([7c062a68](google/bigframes@7c062a68)) * pivot_table supports fill_value arg (#2257) ([8f490e68](google/bigframes@8f490e68)) * Support builtins funcs for df.agg (#2256) ([956a5b00](google/bigframes@956a5b00)) * add bigquery.json_keys (#2286) ([b487cf1f](google/bigframes@b487cf1f)) * Add agg/aggregate methods to windows (#2288) ([c4cb39dc](google/bigframes@c4cb39dc)) * Add bigframes.pandas.crosstab (#2231) ([c62e5535](google/bigframes@c62e5535)) * Implement single-column sorting for interactive table widget (#2255) ([d1ecc61b](google/bigframes@d1ecc61b)) ### Bug Fixes * Pass credentials properly for read api instantiation (#2280) ([3e3fe259](google/bigframes@3e3fe259)) * Update max_instances default to reflect actual value (#2302) ([4489687e](google/bigframes@4489687e)) * Improve Anywidget pagination and display for unknown row counts (#2258) ([508deae5](google/bigframes@508deae5)) * Fix issue with stream upload batch size upload limit (#2290) ([6cdf64b0](google/bigframes@6cdf64b0)) * calling info() on empty dataframes no longer leads to errors (#2267) ([95a83f77](google/bigframes@95a83f77)) * do not warn with DefaultIndexWarning in partial ordering mode (#2230) ([cc2dbae6](google/bigframes@cc2dbae6)) ### Documentation * update docs and tests for Gemini 2.5 models (#2279) ([08c0c0c8](google/bigframes@08c0c0c8)) * Add Google Analytics configuration to conf.py (#2301) ([0b266da1](google/bigframes@0b266da1)) * fix LogisticRegression docs rendering (#2295) ([32e53134](google/bigframes@32e53134)) * update API reference to new `dataframes.bigquery.dev` location (#2293) ([da064397](google/bigframes@da064397)) * use autosummary to split documentation pages (#2251) ([f7fd2d20](google/bigframes@f7fd2d20)) </details>
| Back | FazBrowse Home | New Git URL |
This is not just "taking the easy way out", it's really the most appropriate fix since there are so many
assertions that can fail due to eventual consistency. (For example, asking for 2 blobs should have a next page when 3 are in the bucket, but this may break down due to eventual consistency.)
Fixes #2217.
Also
@tseaver I would like some input here. I am worried that RetryErrors on the error unittest.TestCase.failureException is insufficient / overly general. This is especially because
unittest.TestCase.failureException is just an alias for AssertionError.
Should we temporarily replace it (via another decorator)?