As nielm pointed out in his comment of my PR, I added this because
"spanner API change that exposes Guava classes is:
googleapis/java-spanner/pull/81,
Specifically, adding AsyncResultSet in
googleapis/java-spanner/pull/81/files#diff-7a9cb34faeb259be46b44f1878b7210f
which returns an ImmutableList."
Without this addition, the ApiSurface test would fail, please see:
beam_PreCommit_Java_Commit #13264 test - testGcpApiSurface [Jenkins]. So I was
suggested to add new exposed class explicitly.
|
|
| |
beam_PreCommit_Java_Commit #13264 test - testGcpApiSurface [Jenkins]
|
|
|
Thanks!
On Tuesday, September 1, 2020, 03:48:37 PM PDT, Chamikara Jayalath
<[email protected]> wrote:
BTW this PR adds the following to the API surface.
(com.google.common.collect.ImmutableCollection.class),
(com.google.common.collect.ImmutableCollection.Builder.class),
(com.google.common.collect.ImmutableList.class),
(com.google.common.collect.ImmutableList.Builder.class),
(com.google.common.collect.UnmodifiableIterator.class),
(com.google.common.collect.UnmodifiableListIterator.class),
Any objections to this ?
Terry, could you explain the reason for adding this.
Thanks,Cham
On Tue, Sep 1, 2020 at 2:40 PM Chamikara Jayalath <[email protected]> wrote:
LGTM. We can merge when tests pass.
Thanks,Cham
On Tue, Sep 1, 2020 at 1:32 PM terry xian <[email protected]> wrote:
Hi,
My pull request [BEAM-8758] Google-cloud-spanner upgrade to 1.59.0 and
google_cloud_bigtable_client_core to 1.16.0 by terryxian78 · Pull Request
#12695 · apache/beam was there for more than 3 days. Although I've added a
reviewer (lukecwik), I am afraid that I missed something which might cause the
PR not noticed (it is my first PR in Beam). I've asked some folks which work on
spanner change review my change but need committee member for approval.
Could someone in committee review my PR?
Thanks!
|
|
|
| | |
|
|
|
| |
[BEAM-8758] Google-cloud-spanner upgrade to 1.59.0 and google_cloud_bigt...
Fixes https://issues.apache.org/jira/browse/BEAM-8758 R: @lukecwik CC: @suztomo
The changes are: The main purpo...
|
|
|
