roryqi commented on code in PR #12420:
URL: https://github.com/apache/gravitino/pull/12420#discussion_r3781558017


##########
api/src/main/java/org/apache/gravitino/SupportsCatalogs.java:
##########
@@ -84,22 +87,66 @@ default boolean catalogExists(String catalogName) {
    * the created catalog is the managed catalog, like model, fileset catalog. 
For the details of the
    * provider definition, see {@link CatalogProvider}.
    *
-   * @param catalogName the name of the catalog.
-   * @param type the type of the catalog.
-   * @param provider the provider of the catalog, or null if the catalog is a 
managed catalog.
-   * @param comment the comment of the catalog.
-   * @param properties the properties of the catalog.
-   * @return The created catalog.
-   * @throws NoSuchMetalakeException If the metalake does not exist.
-   * @throws CatalogAlreadyExistsException If the catalog already exists.
+   * <p>Delegates to {@link #createCatalog(String, Catalog.Type, String, 
String, Map, Map, Map)}
+   * with empty secret maps.
+   *
+   * @param catalogName the name of the catalog
+   * @param type the type of the catalog
+   * @param provider the provider of the catalog, or null if the catalog is a 
managed catalog
+   * @param comment the comment of the catalog
+   * @param properties the properties of the catalog
+   * @return the created catalog
+   * @throws NoSuchMetalakeException if the metalake does not exist
+   * @throws CatalogAlreadyExistsException if the catalog already exists
    */
-  Catalog createCatalog(
+  default Catalog createCatalog(
       String catalogName,
       Catalog.Type type,
       String provider,
       String comment,
       Map<String, String> properties)
-      throws NoSuchMetalakeException, CatalogAlreadyExistsException;
+      throws NoSuchMetalakeException, CatalogAlreadyExistsException {
+    return createCatalog(
+        catalogName,
+        type,
+        provider,
+        comment,
+        properties,
+        Collections.emptyMap(),
+        Collections.emptyMap());
+  }
+
+  /**
+   * Create a catalog with optional secret maps.
+   *
+   * <p>The default implementation rejects create-time secrets. 
Implementations that support secrets
+   * must override this method.
+   *
+   * @param catalogName the name of the catalog
+   * @param type the type of the catalog
+   * @param provider the provider of the catalog, or null if managed
+   * @param comment the comment of the catalog
+   * @param properties the properties of the catalog
+   * @param secretBindings optional property key → binding ({@code provider} + 
{@code plaintext})
+   *     for write-through
+   * @param secretReferences optional property key → secret locator ({@code 
provider} plus
+   *     provider-specific attributes)
+   * @return the created catalog
+   * @throws NoSuchMetalakeException if the metalake does not exist
+   * @throws CatalogAlreadyExistsException if the catalog already exists
+   * @throws UnsupportedOperationException if create-time secrets are not 
supported
+   */
+  default Catalog createCatalog(
+      String catalogName,
+      Catalog.Type type,
+      String provider,
+      String comment,
+      Map<String, String> properties,
+      Map<String, SecretBinding> secretBindings,
+      Map<String, SecretReference> secretReferences)
+      throws NoSuchMetalakeException, CatalogAlreadyExistsException {
+    throw new UnsupportedOperationException("Not implemented");

Review Comment:
   Could u use complete error message here?  For example. `xxxx is not 
implemented`.



##########
clients/client-java/src/main/java/org/apache/gravitino/client/BaseSchemaCatalog.java:
##########
@@ -167,20 +171,35 @@ public String[] listSchemas(String parentSchema)
   }
 
   /**
-   * Create a new schema with specified identifier, comment and metadata.
+   * Create a new schema with specified identifier, comment, properties, and 
optional secret maps.
    *
    * @param schemaName The name identifier of the schema.
    * @param comment The comment of the schema.
    * @param properties The properties of the schema.
+   * @param secretBindings Optional property key → binding ({@code provider} + 
{@code plaintext})
+   *     for write-through.
+   * @param secretReferences Optional property key → secret locator ({@code 
provider} plus
+   *     provider-specific attributes).
    * @return The created {@link Schema}.
    * @throws NoSuchCatalogException if the catalog with specified namespace 
does not exist.
    * @throws SchemaAlreadyExistsException if the schema with specified 
identifier already exists.
    */
   @Override
-  public Schema createSchema(String schemaName, String comment, Map<String, 
String> properties)

Review Comment:
   Do u modify the method signature? We should keep backwards compatibility.



##########
clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java:
##########
@@ -137,9 +139,13 @@ public Catalog createCatalog(
       Catalog.Type type,
       String provider,
       String comment,
-      Map<String, String> properties)
+      Map<String, String> properties,
+      Map<String, SecretBinding> secretBindings,
+      Map<String, SecretReference> secretReferences)
       throws NoSuchMetalakeException, CatalogAlreadyExistsException {
-    return getMetalake().createCatalog(catalogName, type, provider, comment, 
properties);
+    return getMetalake()

Review Comment:
   ditto.



##########
api/src/main/java/org/apache/gravitino/SupportsSchemas.java:
##########
@@ -93,15 +96,48 @@ default boolean schemaExists(String schemaName) {
    * need the schema with default values applied, use the {@link 
#loadSchema(String)} method after
    * creation.
    *
+   * <p>Delegates to {@link #createSchema(String, String, Map, Map, Map)} with 
empty secret maps.
+   *
+   * @param schemaName The name of the schema.
+   * @param comment The comment of the schema.
+   * @param properties The properties of the schema.
+   * @return The schema as defined by the caller, without all default values.
+   * @throws NoSuchCatalogException If the catalog does not exist.
+   * @throws SchemaAlreadyExistsException If the schema already exists.
+   */
+  default Schema createSchema(String schemaName, String comment, Map<String, 
String> properties)
+      throws NoSuchCatalogException, SchemaAlreadyExistsException {
+    return createSchema(
+        schemaName, comment, properties, Collections.emptyMap(), 
Collections.emptyMap());
+  }
+
+  /**
+   * Creates a schema with optional secret maps.
+   *
+   * <p>The default implementation rejects create-time secrets. 
Implementations that support secrets
+   * must override this method.
+   *
    * @param schemaName The name of the schema.
    * @param comment The comment of the schema.
    * @param properties The properties of the schema.
+   * @param secretBindings optional property key → binding ({@code provider} + 
{@code plaintext})
+   *     for write-through
+   * @param secretReferences optional property key → secret locator ({@code 
provider} plus
+   *     provider-specific attributes)
    * @return The schema as defined by the caller, without all default values.
    * @throws NoSuchCatalogException If the catalog does not exist.
    * @throws SchemaAlreadyExistsException If the schema already exists.
+   * @throws UnsupportedOperationException if create-time secrets are not 
supported
    */
-  Schema createSchema(String schemaName, String comment, Map<String, String> 
properties)
-      throws NoSuchCatalogException, SchemaAlreadyExistsException;
+  default Schema createSchema(

Review Comment:
   ditto.



-- 
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]

Reply via email to