luoyuxia commented on code in PR #1937:
URL: https://github.com/apache/fluss/pull/1937#discussion_r2618714151
##########
fluss-common/src/main/java/org/apache/fluss/utils/PropertiesUtils.java:
##########
@@ -57,6 +59,21 @@ public static <V> Map<String, V> extractPrefix(Map<String,
V> originalMap, Strin
.collect(Collectors.toMap(Map.Entry::getKey,
Map.Entry::getValue));
}
+ public static <V> Map<String, V> extractPrefix(
Review Comment:
why introducr a new method? Can't we reuse `extractPrefix(Map<String, V>
originalMap, String prefix)`?
##########
fluss-flink/fluss-flink-common/src/main/java/org/apache/fluss/flink/lake/LakeFlinkCatalog.java:
##########
@@ -69,12 +71,28 @@ public Catalog getLakeCatalog(Configuration tableOptions) {
+
ConfigOptions.TABLE_DATALAKE_FORMAT.key()
+ "' is set.");
}
+ String dataLakePrefix = lakeFormat.toString() + ".";
+ Map<String, String> catalogProperties =
Review Comment:
Can be
```
Map<String, String> catalogProperties =
PropertiesUtils.extractAndRemovePrefix(
lakeCatalogProperties, lakeFormat + ".");
```
?
##########
fluss-common/src/main/java/org/apache/fluss/utils/PropertiesUtils.java:
##########
@@ -57,6 +59,21 @@ public static <V> Map<String, V> extractPrefix(Map<String,
V> originalMap, Strin
.collect(Collectors.toMap(Map.Entry::getKey,
Map.Entry::getValue));
}
+ public static <V> Map<String, V> extractPrefix(
Review Comment:
I suggest to handle in `FlinkCatalogFactory`. The method is hard to reuse...
##########
fluss-common/src/main/java/org/apache/fluss/config/FlussConfigUtils.java:
##########
@@ -45,6 +47,14 @@ public class FlussConfigUtils {
Collections.singletonList(ConfigOptions.TABLE_DATALAKE_ENABLED.key());
}
+ public static final Set<String> SENSITIVE_TABLE_OPTIONS = new HashSet<>();
Review Comment:
why move to here?
##########
fluss-flink/fluss-flink-common/src/main/java/org/apache/fluss/flink/catalog/FlinkCatalog.java:
##########
@@ -115,6 +115,7 @@ public class FlinkCatalog extends AbstractCatalog {
protected final String bootstrapServers;
protected final Map<String, String> securityConfigs;
protected final LakeFlinkCatalog lakeFlinkCatalog;
+ protected final Map<String, String> lakeCatalogProperties;
Review Comment:
Could you please make `lakeCatalogProperties` to be `Supplier<Map<String,
String>> lakeCatalogPropertiesProvider`. The reason it that it make it more
plugable. In our inner use, we will retrieve the lake catalog related from
flink conf, but we only want to do the retrieve if it's required.
##########
fluss-flink/fluss-flink-common/src/test/java/org/apache/fluss/flink/catalog/FlinkCatalogFactoryTest.java:
##########
@@ -75,7 +76,12 @@ public void testCreateCatalog() {
securityMap.put("client.security.sasl.username", "root");
securityMap.put("client.security.sasl.password", "password");
+ Map<String, String> lakeCatalogMap = new HashMap<>();
Review Comment:
add test in `FlinkCatalogITCase` to verify SQL can pass the properties.
--
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]
