hdygxsj commented on code in PR #6543:
URL: https://github.com/apache/gravitino/pull/6543#discussion_r1998804982
##########
flink-connector/flink/src/main/java/org/apache/gravitino/flink/connector/catalog/BaseCatalogFactory.java:
##########
@@ -43,9 +44,13 @@ public interface BaseCatalogFactory extends CatalogFactory {
/**
* Define properties converter {@link PropertiesConverter}.
*
+ * @param catalogOptions Some catalogs (such as {@link
+ * org.apache.gravitino.flink.connector.jdbc.GravitinoJdbcCatalog})
require the use of catalog
+ * options when creating a table, as these options will not be returned
by the
+ * GravitinoServer.
* @return The requested property converter.
*/
- PropertiesConverter propertiesConverter();
+ PropertiesConverter propertiesConverter(Map<String, String> catalogOptions);
Review Comment:
How about splitting the PropertiesConverter into CatalogPropertiesConverter
and TablePropertiesConverter. When loading a catalog, use
CatalogPropertiesConverter to parse the catalog-related properties. Then, when
creating the catalog, pass the converted catalogOptions to
TablePropertiesConverter for further processing. This way, the separation of
concerns might make the logic clearer and more maintainable.
--
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]
