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-6959] add beam_PostCommit_Go_VR_Flink job #8606
Conversation
Run Seed Job |
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.
The different between the "validates runner" and "validates that SDK X works on runner Y" is subtle for sure. The validates runner term is definitely a holdover from the legacy implementations, instead of being precise for portability.
R: @angoenka |
Run Go Flink ValidatesRunner |
f68884a
to
413cdfc
Compare
Run Seed Job |
Run Go Flink ValidatesRunner |
413cdfc
to
d88a451
Compare
Run Seed Job |
Run Go Flink ValidatesRunner |
Hooray, it worked! We even got a shiny badge |
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!
We can also update following get more of those fancy badges
https://github.com/apache/beam/blob/master/README.md
https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md
We can do that in a followup PR.
\o/ Woot! |
Done, although I'm not sure why these manually maintained lists are still around. Shouldn't the process be automated? |
@@ -48,13 +48,13 @@ golang { | |||
} | |||
|
|||
task flinkValidatesRunner { | |||
dependsOn ":beam-sdks-go-test:build" | |||
dependsOn ":beam-runners-flink_2.11-job-server:shadowJar" |
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.
Does the SDK not work on 2.11, or is that version not yet portable? I'm concerned about running on an older version of flink
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.
AFAIK this isn't actually a downgrade, it's just a change in the way we identify Flink runner versions (apparently with Flink versions now instead of Beam) as of [1]. @angoenka can you confirm?
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.
Yes, this is just a naming change to match flink version.
Probably! I agree. @jasonkuster might have ideas around Generating the test grid README's properly based on something more canonical. However, it certainly shouldn't block this from going in. |
@angoenka Is there anything blocking this being merged that must be in this PR? I think there's the one question on the Flink runner versioning that should be answered, but that's it. |
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 @ibzib
@@ -48,13 +48,13 @@ golang { | |||
} | |||
|
|||
task flinkValidatesRunner { | |||
dependsOn ":beam-sdks-go-test:build" | |||
dependsOn ":beam-runners-flink_2.11-job-server:shadowJar" |
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.
Yes, this is just a naming change to match flink version.
Note that I pretty much used "validates runner" and "integration" as consistently as I could within the existing naming scheme, but the distinction is pretty much meaningless as it's the same few tests anyway.
R: @lostluck
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.