Skip to content

Conversation

@vnorigoog
Copy link
Contributor

@vnorigoog vnorigoog commented Mar 20, 2020

Previously, using grpc channel for GCS was controlled by an API change.
Changing that to an experiment, so that there is no need to introduce an API change.
That API change is not in use by anone yet because the underlying change in GCS library to use grpc is not yet public and hence there is no one yet using the new API.


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 SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- Build Status --- --- Build Status
Java Build Status Build Status Build Status
Build Status
Build Status
Build Status
Build Status
Build Status Build Status Build Status
Build Status
Build Status
Python Build Status
Build Status
Build Status
Build Status
--- Build Status
Build Status
Build Status
Build Status
Build Status
--- --- Build Status
XLang --- --- --- Build Status --- --- Build Status

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website
Non-portable Build Status Build Status
Build Status
Build Status Build Status
Portable --- Build Status --- ---

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

@vnorigoog vnorigoog changed the title add experiment flag use_grpc_for_gcs [BEAM-11183]add experiment flag use_grpc_for_gcs Mar 20, 2020
@vnorigoog vnorigoog changed the title [BEAM-11183]add experiment flag use_grpc_for_gcs [BEAM-8889]add experiment flag use_grpc_for_gcs Mar 20, 2020
Copy link
Contributor Author

@vnorigoog vnorigoog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

R: chamikaramj, veblush

@chamikaramj
Copy link
Contributor

Retest this please

@chamikaramj
Copy link
Contributor

LGTM. Thanks.

@chamikaramj
Copy link
Contributor

Run Java PreCommit

@chamikaramj
Copy link
Contributor

Run Java PreCommit

3 similar comments
@lukecwik
Copy link
Member

Run Java PreCommit

@lukecwik
Copy link
Member

Run Java PreCommit

@chamikaramj
Copy link
Contributor

Run Java PreCommit

@lukecwik
Copy link
Member

It looks like spotbugs is failing in the Java PreCommit with:

Dead store to gcsOptions in org.apache.beam.sdk.extensions.gcp.util.GcsUtil$GcsUtilFactory.create(PipelineOptions, Storage, HttpRequestInitializer, ExecutorService, Integer)
Bug type DLS_DEAD_LOCAL_STORE (click for details)
In class org.apache.beam.sdk.extensions.gcp.util.GcsUtil$GcsUtilFactory
In method org.apache.beam.sdk.extensions.gcp.util.GcsUtil$GcsUtilFactory.create(PipelineOptions, Storage, HttpRequestInitializer, ExecutorService, Integer)
Local variable named gcsOptions
At GcsUtil.java:[line 115]

@vnorigoog
Copy link
Contributor Author

vnorigoog commented Apr 1, 2020 via email

@lukecwik
Copy link
Member

lukecwik commented Apr 1, 2020

retest this please

@lukecwik
Copy link
Member

lukecwik commented Apr 1, 2020

Did you run check task of the gradle project to cover spotbugs/checkstyle/unit tests/... since test only covers testing?

Also, I had to run the command locally after checking out your PR since the full error log in Jenkins points to a local file on the Jenkins host which isn't accessible.

@vnorigoog
Copy link
Contributor Author

vnorigoog commented Apr 1, 2020 via email

@lukecwik
Copy link
Member

lukecwik commented Apr 1, 2020

Run JavaPortabilityApi PreCommit

2 similar comments
@lukecwik
Copy link
Member

lukecwik commented Apr 1, 2020

Run JavaPortabilityApi PreCommit

@lukecwik
Copy link
Member

lukecwik commented Apr 1, 2020

Run JavaPortabilityApi PreCommit

@chamikaramj
Copy link
Contributor

New failure seems to be unrelated.

@chamikaramj
Copy link
Contributor

Run JavaPortabilityApi PreCommit

1 similar comment
@lukecwik
Copy link
Member

lukecwik commented Apr 2, 2020

Run JavaPortabilityApi PreCommit

@lukecwik lukecwik merged commit 0a451f2 into apache:master Apr 2, 2020
@veblush veblush mentioned this pull request Apr 4, 2020
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants