-
Notifications
You must be signed in to change notification settings - Fork 4.5k
sharding GCP IO tests from the javaPostCommit task #21800
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
sharding GCP IO tests from the javaPostCommit task #21800
Conversation
|
Can one of the admins verify this patch? |
1 similar comment
|
Can one of the admins verify this patch? |
| timeoutMins: 120, | ||
| triggerPathPatterns: [ | ||
| '^sdks/java/io/google-cloud-platform/.*$', | ||
| '^runners/direct-java/.*$', |
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.
Remove this trigger path, since a change to the direct runner does not change the correctness of the IOs.
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.
Will do!
| '^runners/direct-java/.*$', | ||
| ] | ||
| ) | ||
| builder.build { |
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.
Is this different from the other precommit builder defaults?
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.
Yeah, they are different. I added the paths to the direct runner and gcp io folders.
Thanks Kenn!
|
There a conflict caused by the other sharding PR, should be easy to fix up. Be sure to check spotless. |
|
Hi @damccorm! Do you know who can help us to merge this one? |
|
Yep, I actually can now! |
damccorm
left a comment
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.
LGTM
|
Nice! |
| dependsOn(":sdks:java:extensions:google-cloud-platform-core:postCommit") | ||
| dependsOn(":sdks:java:extensions:zetasketch:postCommit") | ||
| dependsOn(":sdks:java:io:google-cloud-platform:postCommit") | ||
| dependsOn(":sdks:java:io:debezium:integrationTest") |
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.
These line were removed in #21804 but added back in this PR. Sounds like a rebase issue. Should do a rebase instead of merge.
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.
Oh you're right - good catch, I thought that was part of breaking this apart, but it is not. @fernando-wizeline guessing that wasn't intentional?
@Abacn want to put up a PR to remove those lines?
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.
Oh, this is so embarrassing, sorry.
That is correct, it was not intentional.
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.
@damccorm will do
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.
No problem, I should've caught it too! Thanks Yi
Removing the job from the javaPostcommit task and adding its own task for GCP IO tests.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).CHANGES.mdwith noteworthy changes.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)
See CI.md for more information about GitHub Actions CI.