AHeise commented on a change in pull request #17580:
URL: https://github.com/apache/flink/pull/17580#discussion_r741702581
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/minicluster/MiniCluster.java
##########
@@ -279,6 +282,10 @@ public void start() throws Exception {
miniClusterConfiguration.getRpcServiceSharing() ==
RpcServiceSharing.SHARED;
try {
+ final PluginManager pluginManager =
+
PluginUtils.createPluginManagerFromRootFolder(configuration);
Review comment:
Why is `null` sufficient? I think the `pluginManager` is needed such
that files in the plugin folder are loaded at all. That's the only way to
read/write to S3 in local setup. I'm probably missing something here.
##########
File path:
flink-clients/src/test/java/org/apache/flink/client/ExecutionEnvironmentTest.java
##########
@@ -26,13 +26,22 @@
import org.apache.flink.api.java.io.DiscardingOutputFormat;
Review comment:
Btw same question as below: have we actually initialized plugins before
your fix at all? I don't think so.
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/minicluster/MiniCluster.java
##########
@@ -279,6 +282,10 @@ public void start() throws Exception {
miniClusterConfiguration.getRpcServiceSharing() ==
RpcServiceSharing.SHARED;
try {
+ final PluginManager pluginManager =
+
PluginUtils.createPluginManagerFromRootFolder(configuration);
+ FileSystem.initialize(configuration, pluginManager);
Review comment:
How about we add a test-only `uninitialize`? Wouldn't that be the
cleanest option?
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/minicluster/MiniCluster.java
##########
@@ -279,6 +282,10 @@ public void start() throws Exception {
miniClusterConfiguration.getRpcServiceSharing() ==
RpcServiceSharing.SHARED;
try {
+ final PluginManager pluginManager =
+
PluginUtils.createPluginManagerFromRootFolder(configuration);
+ FileSystem.initialize(configuration, pluginManager);
Review comment:
Every test that calls `initialize` must call `uninitialize`. We can't
enforce it but rather make sure that all existing tests are abiding it.
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/minicluster/MiniCluster.java
##########
@@ -279,6 +282,10 @@ public void start() throws Exception {
miniClusterConfiguration.getRpcServiceSharing() ==
RpcServiceSharing.SHARED;
try {
+ final PluginManager pluginManager =
+
PluginUtils.createPluginManagerFromRootFolder(configuration);
Review comment:
Yes, but users are trying out their Flink job locally and use a `plugin`
folder in their working directories for s3 (that works by default).
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/minicluster/MiniCluster.java
##########
@@ -279,6 +282,10 @@ public void start() throws Exception {
miniClusterConfiguration.getRpcServiceSharing() ==
RpcServiceSharing.SHARED;
try {
+ final PluginManager pluginManager =
+
PluginUtils.createPluginManagerFromRootFolder(configuration);
Review comment:
Just from the last month:
https://lists.apache.org/thread/kojvkx9ops9fmdkkcwsmpzkwrx94wcym
https://lists.apache.org/thread/2dnlk7lc489ts20338cl3ll8gvfx6f21
--
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]
