cshannon commented on code in PR #3506:
URL: https://github.com/apache/accumulo/pull/3506#discussion_r1240724372


##########
server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java:
##########
@@ -128,6 +129,23 @@ private static ArrayList<Short> 
addIfNotPresent(ArrayList<Short> lst, int intVio
     return lst;
   }
 
+  /*
+   * Validates the data file metadata by creating a StoredTabletFile for the 
metadata string that
+   * will eagerly load the contained ReferencedTabletFile which performs 
validation.
+   *
+   * We have to catch a generic Exception as various errors can occur including
+   * NullPointerException, IllegalArgumentException, etc.
+   */
+  private static ArrayList<Short> validateDataFilePath(ArrayList<Short> 
violations,
+      String metadata) {
+    try {
+      new StoredTabletFile(metadata);

Review Comment:
   I actually went to try and add a static validate and noticed there was an 
existing validate method that isn't used anywhere going back to 2.1 so I 
created a PR to remove that first: https://github.com/apache/accumulo/pull/3540
   
   Also, the one downside if we do the validation somewhere else (like a static 
validate method) vs the constructor is potentially an issue with different 
validation being done in different code paths (constructor could end up getting 
out of sync with the static method) but hopefully if someone is changing the 
StoredTabletFile class they would make sure that doesn't happen.



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