[GitHub] [accumulo] ddanielr commented on a diff in pull request #3255: Convert Bulk Import metadata actions to use Ample

["ddanielr (via GitHub)" <gi...@apache.org>]

ddanielr commented on code in PR #3255:
URL: https://github.com/apache/accumulo/pull/3255#discussion_r1147923288


##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java:
##########
@@ -339,4 +339,32 @@ default void deleteScanServerFileReferences(Collection<ScanServerRefTabletFile>
   default void deleteScanServerFileReferences(String serverAddress, UUID serverSessionId) {
     throw new UnsupportedOperationException();
   }
+
+  /**
+   * Create a Bulk Load In Progress flag in the metadata table
+   *
+   * @param path The bulk directory filepath
+   * @param fateTxid The id of the Bulk Import Fate operation.
+   */
+  default void addBulkLoadInProgressFlag(String path, long fateTxid) {
+    throw new RuntimeException();

Review Comment:
   I used `Runtime` since it's a base exception on the interface and should allow more variations in thrown exceptions vs `UnsupportedOperation`.  However, I'm happy to switch it if the goal is to standardize the interface on "Unsupported Operation". 
   
   This brings up a question though about current implementations of the Ample interface. 
   
   This method signature is defined with throwing an `UnsupportedOperation` exception type.
   https://github.com/apache/accumulo/blob/491497435d57c609b10e60d089c211a742b48a09/core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java#L192-L194
   
   However, the implementation of that method only throws a RuntimeException
   https://github.com/apache/accumulo/blob/491497435d57c609b10e60d089c211a742b48a09/server/base/src/main/java/org/apache/accumulo/server/metadata/ServerAmpleImpl.java#L145-L148
   
   `UnsupportedOperationException` extends `RuntimeException` so shouldn't an error be thrown that a subclass is defined in the interface, but a parent class is being thrown in the implementation? 
   
   



-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org