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


##########
clients/client-python/gravitino/dto/requests/fileset_create_request.py:
##########
@@ -45,12 +52,16 @@ def __init__(
         fileset_type: Fileset.Type = None,
         storage_locations: Optional[Dict[str, str]] = None,
         properties: Optional[Dict[str, str]] = None,
+        secret_bindings: Optional[Dict[str, SecretBinding]] = None,

Review Comment:
   ditto.



##########
common/src/main/java/org/apache/gravitino/dto/requests/FilesetCreateRequest.java:
##########
@@ -64,6 +67,36 @@ public class FilesetCreateRequest implements RESTRequest {
   @JsonProperty("properties")
   private Map<String, String> properties;
 
+  @Nullable
+  @JsonInclude(JsonInclude.Include.NON_EMPTY)
+  @JsonProperty("secretBindings")
+  private Map<String, SecretBindingDTO> secretBindings;
+
+  @Nullable
+  @JsonInclude(JsonInclude.Include.NON_EMPTY)
+  @JsonProperty("secretReferences")
+  private Map<String, SecretReferenceDTO> secretReferences;
+
+  /**
+   * Constructor for FilesetCreateRequest without secret maps.
+   *
+   * @param name The name of the fileset.
+   * @param comment The comment of the fileset.
+   * @param type The type of the fileset.
+   * @param storageLocation The storage location of the fileset.
+   * @param storageLocations The storage locations of the fileset.
+   * @param properties The properties of the fileset.
+   */
+  public FilesetCreateRequest(
+      String name,
+      String comment,
+      Fileset.Type type,
+      String storageLocation,
+      Map<String, String> storageLocations,
+      Map<String, String> properties) {
+    this(name, comment, type, storageLocation, storageLocations, properties, 
null, null);

Review Comment:
   Could u use empty collections?



##########
clients/client-python/gravitino/client/fileset_catalog.py:
##########
@@ -176,6 +189,8 @@ def create_multiple_location_fileset(
         fileset_type: Fileset.Type,
         storage_locations: Dict[str, str],
         properties: Dict[str, str],
+        secret_bindings: Optional[Dict[str, SecretBinding]] = None,

Review Comment:
   ditto.



##########
clients/client-python/gravitino/client/fileset_catalog.py:
##########
@@ -138,6 +139,8 @@ def create_fileset(
         fileset_type: Fileset.Type,
         storage_location: str,
         properties: Dict[str, str],
+        secret_bindings: Optional[Dict[str, SecretBinding]] = None,

Review Comment:
   Could u use empty collection?



##########
core/src/main/java/org/apache/gravitino/hook/FilesetHookDispatcher.java:
##########
@@ -75,27 +80,42 @@ public Fileset createMultipleLocationFileset(
       String comment,
       Fileset.Type type,
       Map<String, String> storageLocations,
-      Map<String, String> properties)
+      Map<String, String> properties,
+      Map<String, SecretBinding> secretBindings,
+      Map<String, SecretReference> secretReferences)
       throws NoSuchSchemaException, FilesetAlreadyExistsException {
     Fileset fileset =
         dispatcher.createMultipleLocationFileset(
-            ident, comment, type, storageLocations, properties);
+            ident, comment, type, storageLocations, properties, 
secretBindings, secretReferences);
 
-    // Set the creator as the owner of the fileset.
-    OwnerDispatcher ownerManager = 
GravitinoEnv.getInstance().ownerDispatcher();
-    if (ownerManager != null) {
-      // The inner NormalizeDispatcher case-folds the fileset name (and its 
schema namespace)
-      // based on catalog capabilities, so the entity is stored under the 
normalized identifier.
-      // Apply the same normalization here so the owner is attached to the 
same identifier the
-      // manager sees.
-      NameIdentifier normalizedIdent =
-          CapabilityHelpers.applyCapabilities(
-              ident, Capability.Scope.FILESET, 
GravitinoEnv.getInstance().catalogManager());
-      ownerManager.setOwner(
-          normalizedIdent.namespace().level(0),
-          NameIdentifierUtil.toMetadataObject(normalizedIdent, 
Entity.EntityType.FILESET),
-          PrincipalUtils.getCurrentUserName(),
-          Owner.Type.USER);
+    try {
+      // Set the creator as the owner of the fileset.
+      OwnerDispatcher ownerManager = 
GravitinoEnv.getInstance().ownerDispatcher();
+      if (ownerManager != null) {
+        // The inner NormalizeDispatcher case-folds the fileset name (and its 
schema namespace)
+        // based on catalog capabilities, so the entity is stored under the 
normalized identifier.
+        // Apply the same normalization here so the owner is attached to the 
same identifier the
+        // manager sees.
+        NameIdentifier normalizedIdent =
+            CapabilityHelpers.applyCapabilities(
+                ident, Capability.Scope.FILESET, 
GravitinoEnv.getInstance().catalogManager());
+        ownerManager.setOwner(
+            normalizedIdent.namespace().level(0),
+            NameIdentifierUtil.toMetadataObject(normalizedIdent, 
Entity.EntityType.FILESET),
+            PrincipalUtils.getCurrentUserName(),
+            Owner.Type.USER);
+      }
+    } catch (Exception postHookException) {
+      LOG.warn(
+          "Failed to execute post hook operations, rolling back fileset " + 
ident,
+          postHookException);
+      try {
+        dispatcher.dropFileset(ident);

Review Comment:
   Why do we add extra operations about file set rollback?



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