Airblader commented on a change in pull request #15245:
URL: https://github.com/apache/flink/pull/15245#discussion_r595835599
##########
File path:
flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java
##########
@@ -19,22 +19,103 @@
package org.apache.flink.table.factories;
import org.apache.flink.annotation.PublicEvolving;
+import org.apache.flink.configuration.ConfigOption;
+import org.apache.flink.configuration.ReadableConfig;
import org.apache.flink.table.catalog.Catalog;
+import org.apache.flink.table.catalog.exceptions.CatalogException;
+import java.util.List;
import java.util.Map;
+import java.util.Set;
/**
* A factory to create configured catalog instances based on string-based
properties. See also
- * {@link TableFactory} for more information.
+ * {@link Factory} for more information.
+ *
+ * <p>Note that this interface supports the {@link TableFactory} stack for
compatibility purposes.
+ * This is deprecated, however, and new implementations should implement the
{@link Factory} stack
+ * instead.
*/
@PublicEvolving
-public interface CatalogFactory extends TableFactory {
+public interface CatalogFactory extends TableFactory, Factory {
/**
* Creates and configures a {@link Catalog} using the given properties.
*
* @param properties normalized properties describing an external catalog.
* @return the configured catalog.
+ * @deprecated Use {@link this#createCatalog(Context)} instead and
implement {@link Factory}
+ * instead of {@link TableFactory}.
+ */
+ @Deprecated
+ default Catalog createCatalog(String name, Map<String, String> properties)
{
+ throw new CatalogException("Catalog factories must implement
createCatalog()");
+ }
+
+ /**
+ * Creates and configures a {@link Catalog} using the given context.
+ *
+ * <p>An implementation should perform validation and the discovery of
further (nested)
+ * factories in this method.
*/
- Catalog createCatalog(String name, Map<String, String> properties);
+ default Catalog createCatalog(Context context) {
+ throw new CatalogException("Catalog factories must implement
createCatalog(Context)");
+ }
+
+ /** Context provided when a catalog is created. */
+ interface Context {
+ /** Returns the name with which the catalog is created. */
+ String getName();
+
+ /**
+ * Returns the properties with which the catalog is created.
+ *
+ * <p>An implementation should perform validation of these properties.
+ */
+ Map<String, String> getProperties();
Review comment:
> we also call it options at other locations
The `Factory` interface exposes a `supportedProperties`, though, and these
two are in direct correlation. The usage of properties vs. options seems
inconsistent to me. Is there a clear definition of what the difference is, or
is it the same and we just want to rename it?
----------------------------------------------------------------
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]
