Support google-cloud-spanner v3 and fixes broken unit tests #23365

Abacn · 2022-09-25T03:53:43Z

The native python spannerio was proposed to be deprecated by Q4 but not happens. Still need some maintenance.

Fixes #20529
Fixes #21198

Fix spannerio read unit test actually not assert anything
Loose upperbound of spanner client library

Please add a meaningful description for your change here

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) and mention them in a comment (R: @username).
Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

Abacn · 2022-09-25T03:54:45Z

Run SpannerIO Read 2GB Performance Test Python

Abacn · 2022-09-25T03:56:53Z

Run Python 3.7 PostCommit

codecov · 2022-09-25T04:14:17Z

Codecov Report

Merging #23365 (5b42e71) into master (19f5e62) will increase coverage by 0.01%.
The diff coverage is 66.66%.

❗ Current head 5b42e71 differs from pull request most recent head 55cc71e. Consider uploading reports for the commit 55cc71e to get more accurate results

@@            Coverage Diff             @@
##           master   #23365      +/-   ##
==========================================
+ Coverage   73.40%   73.42%   +0.01%     
==========================================
  Files         718      718              
  Lines       95620    95652      +32     
==========================================
+ Hits        70194    70230      +36     
+ Misses      24115    24111       -4     
  Partials     1311     1311

Flag	Coverage Δ
python	`83.18% <66.66%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
sdks/python/apache_beam/metrics/execution.py	`87.96% <ø> (ø)`
...ython/apache_beam/io/gcp/experimental/spannerio.py	`86.93% <66.66%> (+4.78%)`	⬆️
...s/interactive/dataproc/dataproc_cluster_manager.py	`71.72% <0.00%> (-5.70%)`	⬇️
sdks/python/apache_beam/utils/interactive_utils.py	`95.12% <0.00%> (-2.44%)`	⬇️
...eam/runners/portability/fn_api_runner/execution.py	`92.44% <0.00%> (-0.65%)`	⬇️
...pache_beam/runners/interactive/interactive_beam.py	`81.70% <0.00%> (-0.64%)`	⬇️
sdks/python/apache_beam/transforms/combiners.py	`93.05% <0.00%> (-0.39%)`	⬇️
...eam/runners/interactive/interactive_environment.py	`91.71% <0.00%> (-0.31%)`	⬇️
sdks/python/apache_beam/metrics/cells.py	`82.11% <0.00%> (ø)`
sdks/python/apache_beam/transforms/util.py	`96.06% <0.00%> (ø)`
... and 6 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

github-actions · 2022-09-25T05:19:34Z

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @yeandy for label python.
R: @chamikaramj for label io.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

Abacn · 2022-09-25T13:24:51Z

Run SpannerIO Read 2GB Performance Test Python

Abacn · 2022-09-25T13:25:04Z

Run Python 3.7 PostCommit

Abacn · 2022-09-25T14:12:19Z

Run SpannerIO Read 2GB Performance Test Python

Abacn · 2022-09-25T14:21:54Z

Run Python 3.7 PostCommit

* Fix spannerio read unit test actually not assert anything * Loose upperbound of spanner client library

Abacn · 2022-09-25T16:49:51Z

Run SpannerIO Read 2GB Performance Test Python

Abacn · 2022-09-25T16:55:51Z

Run Python 3.7 PostCommit

Abacn · 2022-09-25T18:57:23Z

apache_beam/runners/worker/sdk_worker_test.py::SdkWorkerTest::test_harness_monitoring_infos_and_metadata
will fail if runs after
apache_beam/io/gcp/experimental/spannerio_test.py::SpannerReadTest::test_read_with_index
or
apache_beam/io/gcp/experimental/spannerio_test.py::SpannerReadTest::test_read_with_table_batch

Looks like not appropriate teardown/setup for tests

Abacn · 2022-09-25T19:18:19Z

Run Python 3.7 PostCommit

Abacn · 2022-09-25T19:20:22Z

Run SpannerIO Read 2GB Performance Test Python

Abacn · 2022-09-25T23:21:45Z

Performance test passed: https://ci-beam.apache.org/job/beam_PerformanceTests_SpannerIO_Read_2GB_Python_PR/8/
postcommit tests passed (albeit test suite has been timed out on master): https://ci-beam.apache.org/job/beam_PostCommit_Python37_PR/441/testReport/apache_beam.io.gcp.experimental.spannerio_read_it_test/
https://ci-beam.apache.org/job/beam_PostCommit_Python37_PR/441/testReport/apache_beam.io.gcp.experimental.spannerio_write_it_test/
ready for review

R: @johnjcasey
R: @tvalentyn

github-actions · 2022-09-25T23:22:40Z

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control

johnjcasey

LGTM

tvalentyn · 2022-09-28T17:58:40Z

sdks/python/setup.py

@@ -289,7 +289,8 @@ def get_portability_package_data():
            'google-cloud-bigquery-storage>=2.6.3,<2.14',
            'google-cloud-core>=0.28.1,<3',
            'google-cloud-bigtable>=0.31.1,<2',
-            'google-cloud-spanner>=1.13.0,<2',
+            # google-cloud-spanner 2.x causes dependency parsing backoff
+            'google-cloud-spanner>=1.13.0,!=2,<=3.21.0',


We shouldn't use <= in the upper bound, it precludes us from asking someone to issue a patch release that could still be picked up by the constraints.
So we can use <3.22.0 or <4.

I think this is server-side dependency only, so using <4 should be safe as we do with other GCP dependencies at this time. We'll decide soon whether we want to cap upper bounds tighter for all IO.

Can you please run PostCommit integration tests with google-cloud-spanner>=3.0.0 requirement if you haven't ? Unless we specify that requirement, postcommit tests will not pick up the new dependency. We can still merge the PR as is with both V1 and V3 bounds. It's also fine to switch the lower bound to V3, as going forward we won't be testing V1.

(1,2) Yes we should set 'google-cloud-spanner<4' as other cloud dependencies

(3) I ran this when debugging (got some error and fixed it). From logging it shows version 3.21.0 is picked by pypi:

15:20:37 Collecting google-cloud-spanner!=2,<=3.21.0,>=1.13.0 15:20:37 Using cached google_cloud_spanner-3.21.0-py2.py3-none-any.whl (291 kB)

also

installed: apache-beam @ file:///app/sdks/python/target/.tox/.tmp/package/1/apache-beam-2.43.0.dev0.zip,cachetools==4.2.4,...,google-cloud-recommendations-ai==0.7.1,google-cloud-spanner==3.21.0,...

jenkins already cached v3.21.0 and is using it.

Thanks, change applied.

tvalentyn · 2022-09-28T17:59:00Z

cc: @BjornPrime

Abacn · 2022-09-28T19:14:02Z

Run Python 3.7 PostCommit

Abacn · 2022-09-28T19:14:11Z

Run SpannerIO Read 2GB Performance Test Python

Abacn · 2022-09-28T23:25:54Z

Integration test and performance test passed with cloud-spanner v3.22.0. Could revert back the last commit

This reverts commit 5b42e71.

github-actions bot added gcp io python labels Sep 25, 2022

github-actions bot added the Next Action: Reviewers label Sep 25, 2022

Python SpannerIO Fixes

9395a96

* Fix spannerio read unit test actually not assert anything * Loose upperbound of spanner client library

Abacn force-pushed the bumpspannerioversion branch from e11aa43 to 9395a96 Compare September 25, 2022 16:41

Fix test_harness_monitoring_infos_and_metadata actually clear metrics

eb87d9c

johnjcasey approved these changes Sep 26, 2022

View reviewed changes

tvalentyn reviewed Sep 28, 2022

View reviewed changes

Abacn added 2 commits September 28, 2022 14:48

Set google-cloud-spanner major version upper bound

0a7fff4

Test pinning google-cloud-spanner version 3

5b42e71

Revert "Test pinning google-cloud-spanner version 3"

55cc71e

This reverts commit 5b42e71.

tvalentyn approved these changes Sep 28, 2022

View reviewed changes

tvalentyn closed this Sep 28, 2022

tvalentyn reopened this Sep 28, 2022

tvalentyn changed the title ~~Python SpannerIO Fixes~~ Support google-cloud-spanner v3 and fixes broken unit tests Sep 28, 2022

tvalentyn merged commit 1427b7d into apache:master Sep 28, 2022

Abacn deleted the bumpspannerioversion branch September 28, 2022 23:56

robertwb mentioned this pull request Sep 30, 2022

[BEAM-10569] Auto-convert spanner tests. #12360

Closed

4 tasks

Abacn mentioned this pull request Oct 4, 2022

Bump google-cloud-spanner version for py containers #23480

Merged

4 tasks

tvalentyn mentioned this pull request Oct 11, 2022

[Bug]: google-cloud-vision 3.x doesn't work in Beam #23261

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support google-cloud-spanner v3 and fixes broken unit tests #23365

Support google-cloud-spanner v3 and fixes broken unit tests #23365

Abacn commented Sep 25, 2022 •

edited

Loading

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

codecov bot commented Sep 25, 2022 •

edited

Loading

github-actions bot commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

github-actions bot commented Sep 25, 2022

johnjcasey left a comment

tvalentyn Sep 28, 2022 •

edited

Loading

Abacn Sep 28, 2022

Abacn Sep 28, 2022

tvalentyn commented Sep 28, 2022

Abacn commented Sep 28, 2022

Abacn commented Sep 28, 2022

Abacn commented Sep 28, 2022 •

edited

Loading

Support google-cloud-spanner v3 and fixes broken unit tests #23365

Support google-cloud-spanner v3 and fixes broken unit tests #23365

Conversation

Abacn commented Sep 25, 2022 • edited Loading

GitHub Actions Tests Status (on master branch)

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

codecov bot commented Sep 25, 2022 • edited Loading

Codecov Report

github-actions bot commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

Abacn commented Sep 25, 2022

github-actions bot commented Sep 25, 2022

johnjcasey left a comment

Choose a reason for hiding this comment

tvalentyn Sep 28, 2022 • edited Loading

Choose a reason for hiding this comment

Abacn Sep 28, 2022

Choose a reason for hiding this comment

Abacn Sep 28, 2022

Choose a reason for hiding this comment

tvalentyn commented Sep 28, 2022

Abacn commented Sep 28, 2022

Abacn commented Sep 28, 2022

Abacn commented Sep 28, 2022 • edited Loading

Abacn commented Sep 25, 2022 •

edited

Loading

codecov bot commented Sep 25, 2022 •

edited

Loading

tvalentyn Sep 28, 2022 •

edited

Loading

Abacn commented Sep 28, 2022 •

edited

Loading