Skip to content
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

Add streaming test for Write API sink #21903

Merged
merged 7 commits into from
Sep 13, 2022
Merged

Conversation

AlexZMLyu
Copy link
Contributor

@AlexZMLyu AlexZMLyu commented Jun 15, 2022

Add streaming test for BigQuery Write 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).
  • Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
  • 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.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests

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

@asf-ci
Copy link

asf-ci commented Jun 15, 2022

Can one of the admins verify this patch?

4 similar comments
@asf-ci
Copy link

asf-ci commented Jun 15, 2022

Can one of the admins verify this patch?

@asf-ci
Copy link

asf-ci commented Jun 15, 2022

Can one of the admins verify this patch?

@asf-ci
Copy link

asf-ci commented Jun 15, 2022

Can one of the admins verify this patch?

@asf-ci
Copy link

asf-ci commented Jun 15, 2022

Can one of the admins verify this patch?

@AlexZMLyu
Copy link
Contributor Author

@chamikaramj Hi Cham, please kindly review the patch.

@yirutang
Copy link
Contributor

R: @chamikaramj

@AlexZMLyu
Copy link
Contributor Author

R: @pabloem

@pabloem
Copy link
Member

pabloem commented Jun 21, 2022

Run Java PostCommit

@pabloem
Copy link
Member

pabloem commented Jun 21, 2022

@AlexZMLyu please run ./gradlew :sdks:java:io:google-cloud-platform:spotlessApply to format the code and fix spotless issue

@pabloem
Copy link
Member

pabloem commented Jun 21, 2022

Running postcommit that exercises these tests: https://ci-beam.apache.org/job/beam_PostCommit_Java_DataflowV1/1765/

@AlexZMLyu
Copy link
Contributor Author

Talked to @pabloem offline. The TestStream class that I used here as the streaming source is not compatible with Dataflow runner. I will edit the code using the GenerateSequence and overriding the expand method to serve as the streaming source instead.

@robertwb
Copy link
Contributor

TestStream should work fine on Runner v2, and is preferable.

@AlexZMLyu
Copy link
Contributor Author

Robert, thanks for letting me know.
Do I need to change my code to utilize the Runner V2?

@chamikaramj
Copy link
Contributor

This test is disabled for Runner v2 currently:

exclude '**/BigQueryIOStorageWriteIT.class'

Trying to re-enable here: #21814

(you can try it on this PR similarly)

@AlexZMLyu
Copy link
Contributor Author

Run PostCommit_Java_DataflowV2

@AlexZMLyu
Copy link
Contributor Author

There was no failed test in https://ci-beam.apache.org/job/beam_PostCommit_Java_DataflowV2_PR/112/testReport/
Why the Dataflow V2 Java Post Commit Tests failed?

@AlexZMLyu
Copy link
Contributor Author

AlexZMLyu commented Aug 8, 2022

Do I need to wait for approval of PR #21814 to run Runner v2 here?

@AlexZMLyu
Copy link
Contributor Author

Run PostCommit_Java_DataflowV2

@chamikaramj
Copy link
Contributor

Seems like Storage Write API tests are still failing for Runner v2: https://ci-beam.apache.org/job/beam_PostCommit_Java_DataflowV2_PR/111/testReport/junit/org.apache.beam.sdk.io.gcp.bigquery/BigQueryIOStorageWriteIT/testBigQueryStorageWrite30MProto/

So we can just get the Runner v1 version in (this PR).

@chamikaramj
Copy link
Contributor

Retest this please

@chamikaramj
Copy link
Contributor

Run PostCommit_Java_DataflowV1

@chamikaramj
Copy link
Contributor

Run PostCommit_Java_Dataflow

@chamikaramj
Copy link
Contributor

Run Java PreCommit

@chamikaramj
Copy link
Contributor

Seems like the new tests failed the post-commit test suite: https://ci-beam.apache.org/job/beam_PostCommit_Java_DataflowV1_PR/124/

@AlexZMLyu
Copy link
Contributor Author

Then I still need to change the streaming source from TestStream to something else, right?

@chamikaramj
Copy link
Contributor

Yeah, I think it's probably ok to use GenerateSequence (a bounded source) for a simple integration test that use the streaming Runner v1. Separately we should check why this still failed for Runner v2.

Change streaming source type from TestStream to GenerateSequence
@AlexZMLyu
Copy link
Contributor Author

Run PostCommit_Java_DataflowV1

1 similar comment
@AlexZMLyu
Copy link
Contributor Author

Run PostCommit_Java_DataflowV1

@chamikaramj
Copy link
Contributor

Run Spotless PreCommit

@AlexZMLyu
Copy link
Contributor Author

Spotless test passed.

@chamikaramj
Copy link
Contributor

Run Java_GCP_IO_Direct PreCommit

@chamikaramj
Copy link
Contributor

Seems like there's still one failing test suite. Probably unrelated. Retrying.

@AlexZMLyu
Copy link
Contributor Author

Looks like all test passed

@chamikaramj
Copy link
Contributor

Thanks. Merging.

@chamikaramj chamikaramj merged commit 6f25fba into apache:master Sep 13, 2022
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.

6 participants