[GitHub] [flink] rmetzger commented on a diff in pull request #21200: [FLINK-29779] Pass PluginManager into MiniCluster

[GitBox <gi...@apache.org>]

rmetzger commented on code in PR #21200:
URL: https://github.com/apache/flink/pull/21200#discussion_r1011303799


##########
flink-runtime/src/main/java/org/apache/flink/runtime/minicluster/MiniClusterConfiguration.java:
##########
@@ -50,6 +51,8 @@ public class MiniClusterConfiguration {
 
     private final MiniCluster.HaServices haServices;
 
+    @Nullable private final PluginManager pluginManager;

Review Comment:
   I see your concern. It is indeed not intuitive how to initialize a plugin manager, and for test, just passing a directory would be easier.
   My motivation for this change comes from a non-testing scenario, where I first initialize the PluginManager, then I initialize the FileSystems, and finally the MiniCluster so that it can set up the metrics reporters. In that use-case, it makes sense to have the PluginManager lifecycle managed outside of the MiniCluster, to avoid instantiating it twice.
   
   How about adding a comment to the `MiniClusterConfiguration.Builder.setPluginManager()` method about how to initialize the PluginManager for now? This would address your concern about how to initialize it.
   Once somebody has a need for using the MiniCluster with plugins in a unit test with MiniClusterExtension, we can evolve this feature further.



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org