[
https://issues.apache.org/jira/browse/BEAM-8557?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
sunjincheng updated BEAM-8557:
------------------------------
Description:
I think we do not need null check here:
[https://github.com/apache/beam/blob/master/runners/java-fn-execution/src/main/java/org/apache/beam/runners/fnexecution/control/FnApiControlClient.java#L151]
Because before the the `onNext` call, the `Future` already put into the queue
in `handle` method.
I found the test as follows:
{code:java}
@Test
public void testUnknownResponseIgnored() throws Exception{code}
I am do not know why we need test this case? @kennknowles
What do you think?
was:
I think we do not need null check here:
[https://github.com/apache/beam/blob/master/runners/java-fn-execution/src/main/java/org/apache/beam/runners/fnexecution/control/FnApiControlClient.java#L151]
Because before the the `onNext` call, the `Future` already put into the queue
in `handle` method.
I found the test as follows:
```
@Test
public void testUnknownResponseIgnored() throws Exception {
String id = "actualInstruction";
String unknownId = "unknownInstruction";
CompletionStage<BeamFnApi.InstructionResponse> responseFuture =
client.handle(BeamFnApi.InstructionRequest.newBuilder().setInstructionId(id).build());
client
.asResponseObserver()
.onNext(BeamFnApi.InstructionResponse.newBuilder().setInstructionId(unknownId).build());
assertThat(MoreFutures.isDone(responseFuture), is(false));
assertThat(MoreFutures.isCancelled(responseFuture), is(false));
}
```
I am do not know why we need test this case? @kennknowles
What do you think?
> Clean up useless null check.
> ----------------------------
>
> Key: BEAM-8557
> URL: https://issues.apache.org/jira/browse/BEAM-8557
> Project: Beam
> Issue Type: Sub-task
> Components: runner-core, sdk-java-harness
> Reporter: sunjincheng
> Assignee: sunjincheng
> Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> I think we do not need null check here:
> [https://github.com/apache/beam/blob/master/runners/java-fn-execution/src/main/java/org/apache/beam/runners/fnexecution/control/FnApiControlClient.java#L151]
> Because before the the `onNext` call, the `Future` already put into the queue
> in `handle` method.
>
> I found the test as follows:
> {code:java}
> @Test
> public void testUnknownResponseIgnored() throws Exception{code}
> I am do not know why we need test this case? @kennknowles
> What do you think?
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
