[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7524: secondary-storage: delete backedup snapshot dir on delete

[via GitHub]

DaanHoogland commented on code in PR #7524:
URL: https://github.com/apache/cloudstack/pull/7524#discussion_r1205193638


##########
services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java:
##########
@@ -1997,6 +1997,17 @@ private void savePostUploadPSK(String psk) {
         }
     }
 
+    protected String getSnapshotFilepathForDelete(String path, String 
snapshotName) {
+        String finalPath = path + "/*" + snapshotName + "*";
+        if (!path.endsWith(snapshotName)) {
+            return finalPath;
+        }

Review Comment:
   ```suggestion
           if (!path.endsWith(snapshotName)) {
               return path + "/*" + snapshotName + "*";
           }
   ```



##########
services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java:
##########
@@ -1997,6 +1997,17 @@ private void savePostUploadPSK(String psk) {
         }
     }
 
+    protected String getSnapshotFilepathForDelete(String path, String 
snapshotName) {
+        String finalPath = path + "/*" + snapshotName + "*";
+        if (!path.endsWith(snapshotName)) {
+            return finalPath;
+        }
+        if (s_logger.isDebugEnabled()) {
+            s_logger.debug(String.format("Snapshot file %s is present in the 
same name directory %s. Deleting the directory", snapshotName, path));
+        }
+        return path;
+    }

Review Comment:
   @weizhouapache @shwstppr , we are dealing with a `String`, not a 
`File`/directory. I think this is ok, in this place. checking for existence 
should not happen in this method.



-- 
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: commits-unsubscr...@cloudstack.apache.org

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