zentol commented on a change in pull request #15553:
URL: https://github.com/apache/flink/pull/15553#discussion_r611484428



##########
File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/externalresource/ExternalResourceUtils.java
##########
@@ -116,10 +117,25 @@ private ExternalResourceUtils() {
         return externalResourceConfigs;
     }
 
+    /**
+     * Instantiate {@link StaticExternalResourceInfoProvider} for all of 
enabled external resources.
+     */
+    public static ExternalResourceInfoProvider 
createStaticExternalResourceInfoProviderFromConfig(
+            Configuration configuration, PluginManager pluginManager) {
+
+        final Map<String, Long> externalResourceAmountMap =
+                getExternalResourceAmountMap(configuration);
+        LOG.info("Enabled external resources: {}", 
externalResourceAmountMap.keySet());

Review comment:
       I was only concerned with a single process logging the same message for 
this PR.
   
   Overall I think this whole class needs a phrasing/sanity pass, but it didn't 
seem worth it at this time because it is a niche feature at this time.
   For example, an external resource can be logged as being enabled, even if 
the driver could not be found later on. This is obviously misleading, and one 
could argue that we maybe should fail if the resource could not be loaded.
   Similar cases are the amount sanity checks: "The amount of the {} should be 
positive while finding {}. Will ignore that resource."




-- 
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