-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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-9815] Correct go integration test if clauses. #11524
Conversation
Run Go PostCommit |
Run Go PostCommit |
Run Go PostCommit VR Flink |
Run Go PostCommit_VR_Flink |
Run Go PostCommit |
R: @robertwb |
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 fixing this! (I was really tempted to re-write this in Python :)
sdks/go/container/boot.go
Outdated
@@ -104,7 +104,7 @@ func main() { | |||
|
|||
switch len(artifacts) { | |||
case 0: | |||
log.Fatal("No artifacts staged") | |||
log.Printf("No artifacts staged. Attempting to run default \"worker\".") |
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.
Shouldn't this be fatal? There isn't a "default" worker.
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.
I can revert this line. It was mostly to explore if the artifacts got copied to the expected location anyway because the model was discovered that way in the first place.
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, if the double vs. single brackets are consistently used.
Run Go Postcommit |
I'll merge once I sanity check that we're back to "No artifacts" for the Dataflow tests, and otherwise passed. |
"No artifacts staged" state has been verified. Merging. |
String comparisons in the integration test script were not running as expected, meaning dev image pushes were not occuring for dataflow, leading to a separate issue. Doesn't fix dataflow,but brings it back to the "No artifacts staged" error, which requires a dataflow change to fix.
String comparisons in the integration test script were not running as expected, meaning dev image pushes were not occuring for dataflow, leading to a separate issue. Doesn't fix dataflow,but brings it back to the "No artifacts staged" error, which requires a dataflow change to fix.
String comparisons in the integration test script were not running as expected, meaning dev image pushes were not occuring for dataflow.
This doesn't fix dataflow, but it brings it back to the "No artifacts staged" issue it was experiencing before.
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-XXX
with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.CHANGES.md
with noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.