[
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 do not know why we need test this case? I think it would be better if we
throw the Exception for an UnknownResponse, otherwise, this may hidden a
potential bug.
Please correct me if there anything I misunderstand @kennknowles
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:
{code:java}
@Test
public void testUnknownResponseIgnored() throws Exception{code}
I do not know why we need test this case? I think it would be better if we
throw the Exception for an UnknownResponse, otherwise, this may hidden a
potential bug.
What do you think? @kennknowles
> 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 do not know why we need test this case? I think it would be better if we
> throw the Exception for an UnknownResponse, otherwise, this may hidden a
> potential bug.
> Please correct me if there anything I misunderstand @kennknowles
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
