-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BEAM-11648] Resubmit Storage API sink with broken test removed. #14309
Conversation
Run Java PreCommit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for removing the test. This PR is ready to merge once all tests pass.
Run Java_Examples_Dataflow PreCommit |
run dataflow streaming validatesrunner |
@@ -490,6 +518,7 @@ public Job getJob(JobReference jobRef, Sleeper sleeper, BackOff backoff) | |||
private DatasetServiceImpl(BigQueryOptions bqOptions) { | |||
this.errorExtractor = new ApiErrorExtractor(); | |||
this.client = newBigQueryClient(bqOptions).build(); | |||
this.newWriteClient = newBigQueryWriteClient(bqOptions); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're supposed to manage the lifecycle of this client and close it when done. Since this code doesn't do that users see SEVERE
errors (e.g. BEAM-12359) whenever they use BigQueryIO. I filed BEAM-12365 for this.
Wasn't the removed broken test an integration test for the feature? How will we test the feature in an ongoing basis for integration? |
The test was broken and didn't test anything. A better test would be to modify BigQueryTornadoes to test this path, but we will do this in a different PR (first we need to ensure that the testing project is whitelisted by BigQuery).