[BEAM-11205] Libraries-BOM 20.0.0 with gRPC and protobuf #14527

suztomo · 2021-04-13T18:49:28Z

Upgrading the libraries-bom version to 20.0.0 along with gRPC and protobuf.

Changes in mock object setup in failed tests

Some GCP-related tests failed. The latest google-http-client internally calls getStatusCode() twice per HTTP response. This change broke some tests where their mock setups had assumed that the method call occurs only once per request (and the tests were reusing the same mock response object for different responses). I updated the mock setup so that it now prepares different mock response objects for different responses.

Linkage Check

No new linkage errors. https://gist.github.com/suztomo/ac11c74fb24ebb8ac0e11791a8be11a9

Initially it had problem with old gax-httpjson used by google-cloud-bigquerystorage (log). Declaring gax-httpjson as a dependency of google-cloud-platform module fixed the linkage errors.

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).
Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
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.

Post-Commit Tests Status (on master branch)

Lang	ULR	Samza	Twister2
Go	---	---	---
Java
Python	---	---	---
XLang	---	---	---

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website	Whitespace	Typescript
Non-portable
Portable	---			---	---	---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

GitHub Actions Tests Status (on master branch)

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

codecov · 2021-04-13T19:28:17Z

Codecov Report

Merging #14527 (f03a076) into master (faaf646) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #14527      +/-   ##
==========================================
- Coverage   83.62%   83.61%   -0.01%     
==========================================
  Files         445      445              
  Lines       59027    59027              
==========================================
- Hits        49359    49358       -1     
- Misses       9668     9669       +1

Impacted Files	Coverage Δ
.../python/apache_beam/examples/dataframe/taxiride.py
...s/python/apache_beam/io/aws/clients/s3/messages.py
...he_beam/runners/interactive/caching/write_cache.py
...ransforms/transforms_keyword_only_args_test_py3.py
...eam/runners/portability/fn_api_runner/fn_runner.py
...am/examples/snippets/transforms/aggregation/min.py
...on/apache_beam/runners/portability/flink_runner.py
...e_beam/runners/portability/abstract_job_service.py
...rcs/sdks/python/apache_beam/io/hadoopfilesystem.py
...s/python/apache_beam/runners/portability/stager.py
... and 880 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update faaf646...f03a076. Read the comment docs.

suztomo · 2021-04-13T19:35:21Z

Run Java_Examples_Dataflow PreCommit

suztomo · 2021-04-13T19:35:36Z

Run PythonLint PreCommit

suztomo · 2021-04-13T20:05:11Z

Run PythonLint PreCommit

suztomo · 2021-04-13T20:24:11Z

:sdks:java:core:compileTestJava failed:

> Task :sdks:java:core:compileTestJava
/Users/suztomo/beam/sdks/java/core/src/test/java/org/apache/beam/sdk/metrics/MetricsTest.java:450: error: incompatible types: inferred type does not conform to lower bound(s)
    assertThat(
              ^
    inferred: MetricResult<DistributionResult>
    lower bound(s): CAP#1,MetricResult<DistributionResult>
  where CAP#1 is a fresh type-variable:
    CAP#1 extends Object super: MetricResult<DistributionResult> from capture of ? super MetricResult<DistributionResult>

MetricsTest.java:450 has

    assertThat(
        metrics.getDistributions(),
        anyOf(
            // Step names are different for portable and non-portable runners.
            hasItem(
                (Matcher<MetricResult<DistributionResult>>)
                    distributionMinMax(
                        NAMESPACE,
                        "bundle",
                        "MyStep1/ParMultiDo(Anonymous)",
                        10L,
                        40L,
                        isCommitted)),
            hasItem(
                (Matcher<MetricResult<DistributionResult>>)
                    distributionMinMax(
                        NAMESPACE,
                        "bundle",
                        "MyStep1-ParMultiDo-Anonymous-",
                        10L,
                        40L,
                        isCommitted))));

Interestingly, the :sdks:java:core:compileTestJava succeeds 56 times out of 77 attempts (without using build cache):

https://gist.github.com/suztomo/dafc42d93be94822a5c750aec7b5574f

My first time to see such randomness in Java build.

Cham is working on fixing the test.

...oud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImplTest.java

elharo · 2021-04-15T11:15:52Z

...e-cloud-platform-core/src/test/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilTest.java

@@ -502,7 +502,9 @@ public void testGetSizeBytesWhenFileNotFoundBatchRetry() throws Exception {

    // 429: Too many requests, then 200: OK.
    when(mockResponse.getStatusCode()).thenReturn(429, 200);
-    when(mockResponse.getContent()).thenReturn(toStream("error"), toStream(content));
+    // Google-http-client:1.39.1 and above does not read content stream of the error response.


This should not be true unless disconnect is being called somewhere.

Thanks. I'll create a test case for google-http-java-client to verify this.

It turned out that it was mock setup that had wrong assumption that getStatusCode is called once per response.

suztomo · 2021-04-16T00:36:56Z

Run Java Postcommit

suztomo · 2021-04-16T01:06:39Z

Run GoPortable PreCommit

suztomo · 2021-04-16T01:06:45Z

Run Java_Examples_Dataflow PreCommit

suztomo · 2021-04-16T01:06:52Z

Run Python PreCommit

suztomo · 2021-04-16T01:09:16Z

Run PostCommit_Java_Hadoop_Versions

suztomo · 2021-04-16T01:09:24Z

Run PostCommit_Java_Dataflow

suztomo · 2021-04-16T01:09:32Z

Run PostCommit_Java_DataflowV2

suztomo · 2021-04-16T01:10:29Z

Run Java HadoopFormatIO Performance Test

suztomo · 2021-04-16T01:10:37Z

Run BigQueryIO Streaming Performance Test Java

suztomo · 2021-04-16T01:10:57Z

Run Dataflow ValidatesRunner

suztomo · 2021-04-17T00:17:53Z

Run Java_Examples_Dataflow PreCommit

suztomo · 2021-04-17T03:55:40Z

Run PostCommit_Java_DataflowV2

suztomo · 2021-04-17T17:05:26Z

34 successful checks!

suztomo · 2021-04-17T19:25:41Z

Run Java Precommit

Fokko

Thanks for picking this up @suztomo Looking good!

Since google-http-client 1.39.1, it started to call getStatusCode twice per a response. However certain tests assumed that the method was called once per response and reused the same mock response object. The test shouldn't assume getStatusCode is called once per response. Therefore The tests that interact with multiple responses now prepare different mock objects for different responses.

The google-cloud-bigquerystorage 1.12.0 has old gax-httpjson version 0.78.0, which causes dependency conflicts. The libraries-bom 20.0.0 has google-cloud-bigquerystorage 1.17.0 but the version has removed a method used in Beam ( apache#14563). Unfortunatley even 1.17.0 does not have compatible version of gax-httpjson. Therefore, this commit fixes the dependency conflict by declaring newer gax-httpjson (version from the libraries-bom) on the sdks/java/io/google-cloud-platform module.

suztomo · 2021-04-19T23:20:59Z

Run PostCommit_Java_DataflowV2

suztomo · 2021-04-19T23:21:07Z

Run Java PostCommit

suztomo · 2021-04-19T23:21:16Z

Run PostCommit_Java_Dataflow

suztomo · 2021-04-19T23:21:22Z

Run Dataflow ValidatesRunner

suztomo · 2021-04-19T23:21:29Z

Run SQL Postcommit

suztomo · 2021-04-19T23:33:47Z

R: @pabloem
Can you review this PR? It's almost same as #14218 but the mock object setup fix included.

suztomo · 2021-04-19T23:34:32Z

Run PythonDocker PreCommit

suztomo · 2021-04-20T00:19:24Z

Run Java_Examples_Dataflow PreCommit

suztomo · 2021-04-20T00:19:29Z

Run Portable_Python PreCommit

suztomo · 2021-04-20T00:19:33Z

Run PythonDocker PreCommit

pabloem · 2021-04-20T16:54:04Z

Run PostCommit_Java_Dataflow

pabloem

Thanks @suztomo ! LGTM
Let's just make sure that our Java tests are all passing, but feel free to merge after that.

suztomo · 2021-04-20T18:53:00Z

@pabloem Thank you. All Java tests passed. (Portable_Python and PythonDocker have been failing in pull requests in this repo)

suztomo changed the title ~~Libraries-BOM 20.0.0 with gRPC and protobuf~~ [BEAM-11205] Libraries-BOM 20.0.0 with gRPC and protobuf Apr 13, 2021

suztomo force-pushed the libraries-bom_20 branch from 9f7e71f to 8bcc6de Compare April 13, 2021 19:41

suztomo force-pushed the libraries-bom_20 branch from 8bcc6de to 2b36069 Compare April 14, 2021 14:40

This comment has been minimized.

Sign in to view

suztomo force-pushed the libraries-bom_20 branch 3 times, most recently from f1b6d20 to 7fe131c Compare April 15, 2021 00:12

suztomo commented Apr 15, 2021

View reviewed changes

...oud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImplTest.java Outdated Show resolved Hide resolved

suztomo mentioned this pull request Apr 15, 2021

fix: when disconnecting, close the underlying connection before the response InputStream googleapis/google-http-java-client#1315

Merged

elharo reviewed Apr 15, 2021

View reviewed changes

suztomo mentioned this pull request Apr 15, 2021

Test case that fails due to NullPointerException googleapis/google-http-java-client#1346

Closed

suztomo force-pushed the libraries-bom_20 branch 4 times, most recently from 317fbb0 to 779bb77 Compare April 16, 2021 00:04

suztomo marked this pull request as ready for review April 17, 2021 00:19

suztomo mentioned this pull request Apr 19, 2021

[BEAM-11968] Bump com.google.cloud:libraries-bom to 19.0.0 #14218

Closed

4 tasks

Fokko approved these changes Apr 19, 2021

View reviewed changes

suztomo requested review from kennknowles and aaltay April 19, 2021 20:10

suztomo added 3 commits April 19, 2021 19:19

Libraries-BOM 20.0.0 with gRPC and protobuf

3564a88

suztomo force-pushed the libraries-bom_20 branch from dedc6c3 to f03a076 Compare April 19, 2021 23:20

suztomo requested a review from pabloem April 19, 2021 23:33

pabloem approved these changes Apr 20, 2021

View reviewed changes

suztomo merged commit 62df129 into apache:master Apr 20, 2021

suztomo deleted the libraries-bom_20 branch April 20, 2021 18:53

suztomo mentioned this pull request Apr 21, 2021

[BEAM-11960]: Bump bigdataoss to 2.2.0 #14200

Closed

4 tasks

[BEAM-11205] Libraries-BOM 20.0.0 with gRPC and protobuf #14527

[BEAM-11205] Libraries-BOM 20.0.0 with gRPC and protobuf #14527

Conversation

suztomo commented Apr 13, 2021 • edited Loading

Changes in mock object setup in failed tests

Linkage Check

Post-Commit Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

GitHub Actions Tests Status (on master branch)

codecov bot commented Apr 13, 2021 • edited Loading

Codecov Report

suztomo commented Apr 13, 2021

suztomo commented Apr 13, 2021

suztomo commented Apr 13, 2021

suztomo commented Apr 13, 2021 • edited Loading

This comment has been minimized.

elharo Apr 15, 2021

Choose a reason for hiding this comment

suztomo Apr 15, 2021

Choose a reason for hiding this comment

suztomo Apr 15, 2021

Choose a reason for hiding this comment

suztomo commented Apr 16, 2021

suztomo commented Apr 16, 2021

suztomo commented Apr 16, 2021

suztomo commented Apr 16, 2021

suztomo commented Apr 16, 2021

suztomo commented Apr 16, 2021

suztomo commented Apr 16, 2021

suztomo commented Apr 16, 2021

suztomo commented Apr 16, 2021

suztomo commented Apr 16, 2021

suztomo commented Apr 17, 2021

suztomo commented Apr 17, 2021

suztomo commented Apr 17, 2021

suztomo commented Apr 17, 2021

Fokko left a comment

Choose a reason for hiding this comment

suztomo commented Apr 19, 2021

suztomo commented Apr 19, 2021

suztomo commented Apr 19, 2021

suztomo commented Apr 19, 2021

suztomo commented Apr 19, 2021

suztomo commented Apr 19, 2021

suztomo commented Apr 19, 2021

suztomo commented Apr 20, 2021

suztomo commented Apr 20, 2021

suztomo commented Apr 20, 2021

pabloem commented Apr 20, 2021

pabloem left a comment

Choose a reason for hiding this comment

suztomo commented Apr 20, 2021

suztomo commented Apr 13, 2021 •

edited

Loading

codecov bot commented Apr 13, 2021 •

edited

Loading

suztomo commented Apr 13, 2021 •

edited

Loading