[GitHub] [iceberg] rdblue commented on a change in pull request #1565: Flink: move hadoop configuration to Loaders from Source/Sink API

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

rdblue commented on a change in pull request #1565:
URL: https://github.com/apache/iceberg/pull/1565#discussion_r502831618



##########
File path: flink/src/main/java/org/apache/iceberg/flink/FlinkCatalogFactory.java
##########
@@ -62,21 +62,22 @@
   /**
    * Create an Iceberg {@link org.apache.iceberg.catalog.Catalog} loader to be used by this Flink catalog adapter.
    *
-   * @param name    Flink's catalog name
-   * @param options Flink's catalog options
+   * @param name       Flink's catalog name
+   * @param hadoopConf Hadoop configuration for catalog
+   * @param options    Flink's catalog options
    * @return an Iceberg catalog loader
    */
-  protected CatalogLoader createCatalogLoader(String name, Map<String, String> options) {
+  protected CatalogLoader createCatalogLoader(String name, Configuration hadoopConf, Map<String, String> options) {

Review comment:
       It's a little odd that the arguments here are the same as `createCatalog`, but that they are in a different order and use "options" rather than "properties". Should we at least use the same argument order?




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org