Finalize Java Prism Runner #32264
Finalize Java Prism Runner #32264
Conversation
|
R: @lostluck |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
There was a problem hiding this comment.
Regarding #32263, please make it clear whether or not it's using the standard testing practice of the TestPipeline rule. I note that TestPipeline isn't being used in the PrismRunnerTests.
See https://beam.apache.org/documentation/pipelines/test-your-pipeline/#testpipeline
| if (Strings.isNullOrEmpty(prismPipelineOptions.getJobEndpoint())) { | ||
| prismPipelineOptions.setJobEndpoint(DEFAULT_PRISM_ENDPOINT); | ||
| if (Strings.isNullOrEmpty(options.getJobEndpoint())) { | ||
| options.setJobEndpoint(DEFAULT_PRISM_ENDPOINT); |
There was a problem hiding this comment.
The default behavior shouldn't be a port that prism selects, but the one this caller selects for prism. There's no guarantee that 8073 is ever free or remains free.
There was a problem hiding this comment.
That's an easy fix. I can have it select randomly based on the freely available.
| pipeline.run(); | ||
| } | ||
|
|
||
| @Ignore("Unable to find inbound timer receiver for instruction") |
There was a problem hiding this comment.
This was fixed by one of my recent PRs.
| } | ||
|
|
||
| @Ignore |
There was a problem hiding this comment.
Why is this test being ignored?
There was a problem hiding this comment.
Late night pull request is the reason.
| @@ -150,14 +153,12 @@ public Server getServer() { | |||
|
|
|||
| @Override | |||
| public void close() throws Exception { | |||
| LOG.info("Shutting down"); | |||
There was a problem hiding this comment.
We'll want to remove this debug logging at least, as this will apply to all portable Java jobs for each FnAPI service.
It's not clear we actually want to include the changes in this file, as the previous version was more robust and already calling the various shutdown sequences.
It also has a higher blast radius than the other cleanups you're applying.
|
Closed due to reversion need of #32247 |
This PR closes #31967, closes #31935, and closes #31935 with a Java Prism Runner.
Currently there's a limitation that the one has to look at the gradle output for whether the test passed as the Job failure isn't getting caught. See #32263.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
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, commentfixes #<ISSUE NUMBER>instead.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 or the workflows README to see a list of phrases to trigger workflows.