joeyutong commented on code in PR #944:
URL: https://github.com/apache/flink-agents/pull/944#discussion_r3759218796
##########
runtime/src/main/java/org/apache/flink/agents/runtime/operator/PythonBridgeManager.java:
##########
@@ -291,14 +294,11 @@ boolean isInitialized() {
@Override
public void close() throws Exception {
- if (pythonActionExecutor != null) {
- pythonActionExecutor.close();
- }
- if (pythonInterpreter != null) {
- pythonInterpreter.close();
- }
- if (pythonEnvironmentManager != null) {
- pythonEnvironmentManager.close();
- }
+ IOUtils.closeAll(
Review Comment:
Thanks for the detailed analysis. I agree that IOUtils.closeAll does not
provide the required behavior for non-Exception Throwables. Since #987 is the
smaller and more general fix, I prefer to let it land first and then rebase
#944 on top of it, adding longTermMemory and pythonResourceAdapter to the
resulting cleanup ladders. I also noticed that #944 introduces another
IOUtils.closeAll inside PythonActionExecutor.close(); I will address that Error
path as part of the rebase.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
