Skip to content

[BEAM-1740] Upgrade to bigtable-client-core 0.9.5.1#2265

Closed
andrewsmartin wants to merge 2 commits intoapache:masterfrom
andrewsmartin:master
Closed

[BEAM-1740] Upgrade to bigtable-client-core 0.9.5.1#2265
andrewsmartin wants to merge 2 commits intoapache:masterfrom
andrewsmartin:master

Conversation

@andrewsmartin
Copy link
Contributor

@andrewsmartin andrewsmartin commented Mar 17, 2017

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

  • Make sure the PR title is formatted like:
    [BEAM-<Jira issue #>] Description of pull request
  • Make sure tests pass via mvn clean verify. (Even better, enable
    Travis-CI on your fork and ensure the whole test matrix passes).
  • Replace <Jira issue #> in the title with the actual Jira issue
    number, if there is one.
  • If this contribution is large, please file an Apache
    Individual Contributor License Agreement.

@andrewsmartin andrewsmartin changed the title [Beam-1740] Upgrade to bigtable-client-core 0.9.5.1 [BEAM-1740] Upgrade to bigtable-client-core 0.9.5.1 Mar 17, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 70.129% when pulling 3860e39 on andrewsmartin:master into b672cde on apache:master.

@asfbot
Copy link

asfbot commented Mar 17, 2017

@davorbonaci
Copy link
Member

R: @dhalperi (welcome back!)

@dhalperi
Copy link
Contributor

LGTM assuming the tests pass!

@tgroh -- any additional changes regarding shading?

@dhalperi
Copy link
Contributor

@davorbonaci @ssisk @jasonkuster I did not see BigtableITs being run in the precommit where they used to be run. Where are they being run now?

@tgroh
Copy link
Member

tgroh commented Mar 20, 2017

No, this should be fine. We may still not be able to shade the Google Cloud Platform IOs without additional changes as bigtable-client-core still exposes Guava on its API surface in return types (specifically in BigtableDataClient with listenable futures and ImmutableList sampleRowKeys(...)

@tgroh
Copy link
Member

tgroh commented Mar 20, 2017

BigtableITs are run in postcommit.

here's one

@dhalperi
Copy link
Contributor

So it looks like this commit breaks those tests. @andrewsmartin can you please take a look? It might be there there has been a change in the version of some Bigtable dependency (gRPC, netty, etc.) https://builds.apache.org/job/beam_PostCommit_Java_MavenInstall/org.apache.beam$beam-sdks-java-io-google-cloud-platform/2959/testReport/junit/org.apache.beam.sdk.io.gcp.bigtable/BigtableReadIT/testE2EBigtableRead/

@dhalperi
Copy link
Contributor

Btw, apparently @andrewsmartin does not have privileges to run the tests without one of the commiters running it for him.

Andrew, I'm assuming you have access to your own Bigtable cluster (if you use BigtableIO) -- suggest running the IT manually in your local developer environment.

@andrewsmartin
Copy link
Contributor Author

@dhalperi You're right, there was a version conflict with netty. This version of the bigtable client relies on a class that only exists in netty-common:4.1.6.Final. I added another commit to bump the netty version project-wide.

@dhalperi
Copy link
Contributor

I kicked off https://builds.apache.org/view/Beam/job/beam_PostCommit_Java_MavenInstall/2972/ to run the ITs manually. Thanks @andrewsmartin

@coveralls
Copy link

Coverage Status

Coverage remained the same at 70.159% when pulling 54393e1 on andrewsmartin:master into 030528f on apache:master.

@asfbot
Copy link

asfbot commented Mar 21, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8632/
--none--

@asfgit asfgit closed this in 7e97820 Mar 21, 2017
@dhalperi
Copy link
Contributor

Passed! Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants