JingsongLi commented on a change in pull request #1332:
URL: https://github.com/apache/iceberg/pull/1332#discussion_r471922290
##########
File path: flink/src/main/java/org/apache/iceberg/flink/FlinkCatalog.java
##########
@@ -80,14 +83,17 @@ public FlinkCatalog(
String catalogName,
String defaultDatabase,
String[] baseNamespace,
- Catalog icebergCatalog,
+ CatalogLoader catalogLoader,
+ Configuration hadoopConf,
boolean cacheEnabled) {
super(catalogName, defaultDatabase);
- this.originalCatalog = icebergCatalog;
- this.icebergCatalog = cacheEnabled ? CachingCatalog.wrap(icebergCatalog) :
icebergCatalog;
+ this.hadoopConf = hadoopConf;
+ this.originalCatalog = catalogLoader.loadCatalog(hadoopConf);
Review comment:
`CachingCatalog` is just a wrapper for original catalog. I think it is
better to not let it implement Closeable interface.
A way to solve this problem can be: Keep `Closeable` as a class member
instead of `Catalog`.
`closeable = originalCatalog instanceof Closeable ? (Closeable)
originalCatalog : null;`
What do you think?
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
