The specification of TearDown is that it is best effort, certainly. 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 >
