kennknowles commented on a change in pull request #13893:
URL: https://github.com/apache/beam/pull/13893#discussion_r571262843



##########
File path: 
sdks/java/harness/src/main/java/org/apache/beam/fn/harness/control/ProcessBundleHandler.java
##########
@@ -705,6 +718,17 @@ void reset() throws Exception {
         resetFunction.run();
       }
     }
+
+    void shutdown() {
+      for (ThrowingRunnable tearDownFunction : getTearDownFunctions()) {
+        LOG.debug("Tearing down function {}", tearDownFunction);
+        try {
+          tearDownFunction.run();
+        } catch (Exception e) {
+          LOG.error("Failed to call teardown function: {}", e);

Review comment:
       Interesting. I did not find good documentation on the expectation about 
whether a bundle should fail if `TearDown` fails. The most detailed docs are 
the 
[javadoc](https://beam.apache.org/releases/javadoc/2.27.0/org/apache/beam/sdk/transforms/DoFn.Teardown.html)
 and they do not describe this. That seems like we need to describe the 
expectation.
   
   Note that I think this will call 
http://www.slf4j.org/apidocs/org/slf4j/Logger.html#error(java.lang.String,java.lang.Throwable)
 because it is the most precise overloaded method.
   
   I think that is good because it will give a stack trace.
   
   But the `{}` will not be replaced with anything.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to