[BEAM-4327] Fix ErrorProne violations in fn-harness #5647
Conversation
|
Seems this needs a rebase (added me as R just because a committer is needed. |
| @@ -81,6 +81,7 @@ private InProcessEnvironmentFactory( | |||
| } | |||
|
|
|||
| @Override | |||
| @SuppressWarnings("FutureReturnValueIgnored") | |||
There was a problem hiding this comment.
Minor: suggest inline document as to why - something along the lines of.
// checked inline in an asynchronous task
There was a problem hiding this comment.
Done; thanks for the suggestion.
| @@ -296,6 +296,7 @@ public void close() throws Exception { | |||
| } | |||
|
|
|||
| @Override | |||
| @SuppressWarnings("FutureReturnValueIgnored") | |||
There was a problem hiding this comment.
Suggest in line comment as to why - perhaps:
// defer error handling on deregistering as that is the responsibility of the beamFnStateClient
|
Looks good @swegner - thanks |
This makes future ErrorProne violations become compile-time errors.
swegner
force-pushed
the
findbugs_fnharness
branch
from
c2d6690
to
b6a4466
Compare
Address PR comments: Add comments for FutureReturnValueIgnored suppressions
|
Done; rebased and added comments for suppressions. This is ready for another round of review. @timrobertson100 please take a look. |
|
@swegner - this is the build failure: https://scans.gradle.com/s/n6bhn6aalygfe/console-log?task=:beam-sdks-java-harness:compileJava#L3 |
Fix compile errors.
swegner
force-pushed
the
findbugs_fnharness
branch
from
4d0b9c6
to
808fff9
Compare
|
Thanks, looks like I missed some things in the merge. I apologize for not building locally first. Compile errors fixed (and tested locally). @timrobertson100 please take a look. |
| @@ -152,6 +159,7 @@ public void tearDown() throws Exception { | |||
| controlClient.close(); | |||
| sdkHarnessExecutor.shutdownNow(); | |||
| serverExecutor.shutdownNow(); | |||
| sdkHarnessExecutorFuture.get(); | |||
There was a problem hiding this comment.
This line is failing the tests due to an InterruptedException
https://scans.gradle.com/s/nihiogx5epq4i/tests/failed
I can reproduce this building your branch with
./gradlew :beam-runners-java-fn-execution:test
Catch InterruptedExceptions in test cleanup
|
Ok, feedback addressed and tests are passing. @timrobertson100 please take a look |
This makes future ErrorProne violations become compile-time errors.
Follow this checklist to help us incorporate your contribution quickly and easily:
[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.It will help us expedite review of your Pull Request if you tag someone (e.g.
@username) to look at it.