-
Notifications
You must be signed in to change notification settings - Fork 4.5k
[BEAM-13857] Switched Go IT script to using Go flags for expansion services #17161
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
Conversation
…rvices This is a followup to a previous PR (apache#16908) which added the feature but didn't switch existing tests to using it. This switches existing tests to using the feature (along with making sure it's set up properly in a way that works).
|
I'd like to note for future reference that the cross-language postcommits for Python Direct, Flink, and Spark all seem to currently be failing, so while running them I'm not going to be checking that they succeed, but that the failures don't change, and we can be reasonably sure that this PR didn't change anything. Since this only affects the way expansion services are initialized for Go, any errors should probably be visible over existing errors. |
|
Run XVR_Direct PostCommit |
|
Run XVR_Flink PostCommit |
|
Run XVR_Spark PostCommit |
Codecov Report
@@ Coverage Diff @@
## master #17161 +/- ##
==========================================
- Coverage 73.96% 73.95% -0.01%
==========================================
Files 671 671
Lines 88245 88245
==========================================
- Hits 65270 65264 -6
- Misses 21863 21869 +6
Partials 1112 1112
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
|
Flink failed with this, rerunning. |
|
Run XVR_Flink PostCommit |
lostluck
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.
Assuming the flink failure is a flake of some kind, this LGTM. Thanks! I wish we had a better/cleaner mechanism to put a prefix like that for testing, but I can't think of anything preferable to the current no-magic approach.
|
Run Flink CrossLanguageValidatesRunner Tests |
|
Run XVR_Flink PostCommit |
|
I'm inclined to think this is a flake for the Flink XVR tests, since this change shouldn't be doing anything to the specific of Flink connections. And looking at the Non-PR version, looks like it's failing with the same error (ex. https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/5215/) Java Precommit I already know is being flaky due to timeouts and this change isn't modifying Java at all. CommunityMetrics Precommit similarly is currently not working for unrelated reasons. |
|
That said, before I merge this I'm going to double check it doesn't conflict with #16819 |
|
Run Flink CrossLanguageValidatesRunner Tests |
|
Run XVR_Flink PostCommit |
|
Run CommunityMetrics PreCommit |
|
Seems like a flake. |
|
Run XVR_Flink PostCommit |
|
Up, just an unrelated flake. Merging. |
This is a followup to a previous PR (#16908) which added the feature but didn't switch existing tests to using it. This switches existing tests to using the feature (along with making sure it's set up properly in a way that works).
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.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.