rmahindra123 commented on code in PR #13836:
URL: https://github.com/apache/hudi/pull/13836#discussion_r2323273706


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/StorageLockClient.java:
##########
@@ -47,4 +47,16 @@ Pair<LockUpsertResult, Option<StorageLockFile>> 
tryUpsertLockFile(
    * @return The lock retrieve result and the current lock file if 
successfully retrieved.
    * */
   Pair<LockGetResult, Option<StorageLockFile>> readCurrentLockFile();
+  
+  /**
+   * Reads a small JSON configuration file from the specified path.
+   * This method is intended for reading small configuration files (e.g., 
audit config, feature flags)
+   * and loads the entire file into memory. Do not use for large files.
+   * 
+   * @param filePath The path to the small JSON config file to read
+   * @param checkExistsFirst If true, performs a lightweight existence check 
before attempting to read.
+   *                         Recommended when the file is unlikely to exist 
(e.g., optional configs).
+   * @return An Option containing the JSON content as a string if successful, 
Option.empty() otherwise
+   */
+  Option<String> readSmallJsonConfig(String filePath, boolean 
checkExistsFirst);

Review Comment:
   shld we rename it to readObject? what is specific here about JSON ?



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/audit/AuditServiceFactory.java:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction.lock.audit;
+
+import org.apache.hudi.client.transaction.lock.StorageLockClient;
+import org.apache.hudi.common.config.TypedProperties;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.storage.StoragePath;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static 
org.apache.hudi.common.table.HoodieTableMetaClient.LOCKS_FOLDER_NAME;
+
+/**
+ * Generic factory for creating audit services.
+ * This factory determines whether auditing is enabled by checking 
configuration files.
+ */
+public class AuditServiceFactory {
+  
+  private static final Logger LOG = 
LoggerFactory.getLogger(AuditServiceFactory.class);
+  private static final String AUDIT_CONFIG_FILE_NAME = "audit_enabled.json";
+  private static final String STORAGE_LP_AUDIT_SERVICE_ENABLED_FIELD = 
"STORAGE_LP_AUDIT_SERVICE_ENABLED";
+  private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
+  
+  /**
+   * Creates a StorageLPAuditService instance by checking the audit 
configuration file.
+   * 
+   * @param properties Configuration properties  
+   * @param ownerId The owner ID for the lock provider
+   * @param basePath The base path of the Hudi table
+   * @param storageLockClient The storage lock client to use for reading 
configuration
+   * @return An Option containing the audit service if enabled, Option.empty() 
otherwise
+   */
+  public static Option<AuditService> createStorageLPAuditService(

Review Comment:
   we can rename to createLockProviderAuditService, lets not use acronyms: LP 
etc. also i feel we can just have a generic name, not Storage ...



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/audit/AuditServiceFactory.java:
##########
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.transaction.lock.audit;
+
+import org.apache.hudi.client.transaction.lock.StorageLockClient;
+import org.apache.hudi.common.config.TypedProperties;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.storage.StoragePath;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static 
org.apache.hudi.common.table.HoodieTableMetaClient.LOCKS_FOLDER_NAME;
+
+/**
+ * Generic factory for creating audit services.
+ * This factory determines whether auditing is enabled by checking 
configuration files.
+ */
+public class AuditServiceFactory {
+  
+  private static final Logger LOG = 
LoggerFactory.getLogger(AuditServiceFactory.class);
+  private static final String AUDIT_CONFIG_FILE_NAME = "audit_enabled.json";
+  private static final String STORAGE_LP_AUDIT_SERVICE_ENABLED_FIELD = 
"STORAGE_LP_AUDIT_SERVICE_ENABLED";
+  private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
+  
+  /**
+   * Creates a StorageLPAuditService instance by checking the audit 
configuration file.
+   * 
+   * @param properties Configuration properties  
+   * @param ownerId The owner ID for the lock provider
+   * @param basePath The base path of the Hudi table
+   * @param storageLockClient The storage lock client to use for reading 
configuration
+   * @return An Option containing the audit service if enabled, Option.empty() 
otherwise
+   */
+  public static Option<AuditService> createStorageLPAuditService(
+      TypedProperties properties, String ownerId, String basePath, 
StorageLockClient storageLockClient) {
+    return createStorageLPAuditService(properties, ownerId, basePath, 
storageLockClient, true);
+  }
+  
+  /**
+   * Creates a StorageLPAuditService instance with optional configuration 
checking.
+   * 
+   * @param properties Configuration properties
+   * @param ownerId The owner ID for the lock provider  
+   * @param basePath The base path of the Hudi table
+   * @param storageLockClient The storage lock client to use for reading 
configuration
+   * @param checkAuditEnabled Whether to check the audit configuration file
+   * @return An Option containing the audit service if enabled, Option.empty() 
otherwise
+   */
+  public static Option<AuditService> createStorageLPAuditService(

Review Comment:
   same comment as above



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