On Mon, May 6, 2019 at 2:06 PM Kenneth Knowles <k...@apache.org> wrote:
> The specification of TearDown is that it is best effort, certainly. > Though I believe the intent of that specification was that a runner will call it as long as the process itself has not crashed. > If your runner supports it, then the test is good to make sure there is > not a regression. If your runner has partial support, that is within spec. > But the idea of the spec is more than you might have such a failure that it > is impossible to call the method, not simply never trying to call it. > > I think it seems to match what we do elsewhere to leave the test, add an > annotation, make a note in the capability matrix about the limitation on > ParDo. > > Kenn > > On Mon, May 6, 2019 at 5:45 AM Michael Luckey <adude3...@gmail.com> wrote: > >> Hi, >> >> after stumbling upon [1] and trying to implement a fix [2], >> ParDoLifeCycleTest are failing for >> direct runner, spark validatesRunnerBatch and flink validatesRunnerBatch >> fail as DoFns teardown is not invoked, if DoFns setup throw an exceptions. >> >> This seems to be in line with the specification [3], as this explicitly >> states that 'teardown might not be called if unnecessary as processed will >> be killed anyway'. >> >> No I am a bit lost on how to resolve this situation. Currently, we seem >> to have following options >> - remove the test, although it seems valuable in different (e.g. >> streaming?) cases >> - to satisfy the test implement the call to teardown in runners although >> it seems unnecessary >> - add another annotation @CallsTeardownAfterFailingSetup, >> @UsesFullParDoLifeCycle or such (would love to get suggestions for >> better name here) >> - ? >> >> Thoughts? >> >> Best, >> >> michel >> >> >> >> [1] https://issues.apache.org/jira/browse/BEAM-7197 >> [2] https://github.com/apache/beam/pull/8495 >> [3] >> https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFn.java#L676-L680 >> >
