SteNicholas commented on pull request #14028:
URL: https://github.com/apache/flink/pull/14028#issuecomment-737624706
> Although the `ProgramInvocationException` is not annotated as `Public` or
`PublicEvolving`, due to the fact that it has been here since forever, it seems
that in some
SteNicholas commented on pull request #14028:
URL: https://github.com/apache/flink/pull/14028#issuecomment-737624582
> example
@kl0u , IMO, I really want to follow the remaining comments from
@tillrohrmann , and continue to update the commit according to Till's above
comments. It's
SteNicholas commented on pull request #14028:
URL: https://github.com/apache/flink/pull/14028#issuecomment-736948175
Thanks for @tillrohrmann detailed review reply. I agree with the point that
`JobExecutionException` transporting the final `ApplicationStatus` shouldn't be
used within the
SteNicholas commented on pull request #14028:
URL: https://github.com/apache/flink/pull/14028#issuecomment-733035458
> Thanks for creating this PR @SteNicholas. In general I like the
unification of thrown exception. I had two questions:
>
> 1. Is the `ProgramInvocationException`
SteNicholas commented on pull request #14028:
URL: https://github.com/apache/flink/pull/14028#issuecomment-733034701
> 2\. ExecutionGraphBuilder
@tillrohrmann
1.`ProgramInvocationException` isn't the part of the public API of a
`JobClient`. Even if it is, it is wrapped into
SteNicholas commented on pull request #14028:
URL: https://github.com/apache/flink/pull/14028#issuecomment-731067152
@tillrohrmann , could you please review this pull request for the
application status of `JobExecutionException?
SteNicholas commented on pull request #14028:
URL: https://github.com/apache/flink/pull/14028#issuecomment-731046963
@tillrohrmann , could you please review this pull request and take a look at
the [commit](https://github.com/kl0u/flink/tree/client-exception-fin) of @kl0u
?
SteNicholas commented on pull request #14028:
URL: https://github.com/apache/flink/pull/14028#issuecomment-728748152
> @SteNicholas I had a look at this solution and I would like to think on
how to make the `status` `final`. This means that I am trying to see if we can
have all the
SteNicholas commented on pull request #14028:
URL: https://github.com/apache/flink/pull/14028#issuecomment-727921163
> @SteNicholas why not putting the `status` in the `JobExecutionException`
and remove the `JobExecutionResultException`?
Because of the compatibility for the previous
SteNicholas commented on pull request #14028:
URL: https://github.com/apache/flink/pull/14028#issuecomment-727142827
> I am having a look right now @SteNicholas
@kl0u Any other comments by you?
This is an automated
SteNicholas commented on pull request #14028:
URL: https://github.com/apache/flink/pull/14028#issuecomment-726555110
> Thanks a lot @SteNicholas. Feel free to take the commit and integrate it
in your PR :)
@kl0u Thanks for you better implementation commit. I have integrated into my
SteNicholas commented on pull request #14028:
URL: https://github.com/apache/flink/pull/14028#issuecomment-725983640
> Hi @SteNicholas , I have a commit with my comments here
https://github.com/apache/flink/tree/client-exception. Also for the tests I
think we should just add tests for all
SteNicholas commented on pull request #14028:
URL: https://github.com/apache/flink/pull/14028#issuecomment-725907605
> Hi @SteNicholas , I reviewing your PR now. One thing to add is some tests
(for each client) that verify that all clients throw the expected type of
exception. This is to
SteNicholas commented on pull request #14028:
URL: https://github.com/apache/flink/pull/14028#issuecomment-725301089
@kl0u , could you please help to review this commit if you are available?
This is an automated message from
14 matches
Mail list logo
