You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "smengcl (via GitHub)" <gi...@apache.org> on 2023/08/11 06:19:40 UTC
[GitHub] [ozone] smengcl opened a new pull request, #5174: HDDS-9157. [Snapshot] Print correct snapshot path in `fs -createSnapshot`
smengcl opened a new pull request, #5174:
URL: https://github.com/apache/ozone/pull/5174
## What changes were proposed in this pull request?
- Correctly print snapshot path when subdirectories are specified during snapshot creation with `fs -createSnapshot`.
cc @hemantk-12
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9157
## How was this patch tested?
- Added new test case in `TestOzoneFsSnapshot`.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #5174: HDDS-9157. [Snapshot] Print correct snapshot path in `fs -createSnapshot`
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #5174:
URL: https://github.com/apache/ozone/pull/5174#discussion_r1290945147
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFsSnapshot.java:
##########
@@ -138,6 +139,39 @@ public void testCreateSnapshotDuplicateName() throws Exception {
Assertions.assertEquals(1, res);
}
+ @Test
+ public void testCreateSnapshotWithSubDirInput() throws Exception {
+ // Test that:
+ // $ ozone fs -createSnapshot ofs://om/vol1/buck2/dir3/ snap1
+ //
+ // should print:
+ // Created snapshot ofs://om/vol1/buck2/.snapshot/snap1
+ //
+ // rather than:
+ // Created snapshot ofs://om/vol1/buck2/dir3/.snapshot/snap1
+
+ String snapshotName = "snap-" + RandomStringUtils.randomNumeric(5);
+
+ String dirPath = BUCKET_PATH + "/dir1/";
+
+ int res = ToolRunner.run(shell, new String[] {
+ "-mkdir", "-p", dirPath});
+ Assertions.assertEquals(0, res);
+
+ try (GenericTestUtils.SystemOutCapturer capture =
+ new GenericTestUtils.SystemOutCapturer()) {
+ res = ToolRunner.run(shell, new String[] {
+ "-createSnapshot", dirPath, snapshotName});
+ // Asserts that create request succeeded
+ Assertions.assertEquals(0, res);
+
+ String expectedSnapshotPath = Paths.get(
+ BUCKET_PATH, OM_SNAPSHOT_INDICATOR, snapshotName).toString();
+ String out = capture.getOutput().trim();
+ Assertions.assertTrue(out.endsWith(expectedSnapshotPath));
+ }
+ }
+
Review Comment:
In this test we relying on log output. Would be better to test the function as well.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a diff in pull request #5174: HDDS-9157. [Snapshot] Print correct snapshot path in `fs -createSnapshot`
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5174:
URL: https://github.com/apache/ozone/pull/5174#discussion_r1291640285
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFsSnapshot.java:
##########
@@ -138,6 +139,39 @@ public void testCreateSnapshotDuplicateName() throws Exception {
Assertions.assertEquals(1, res);
}
+ @Test
+ public void testCreateSnapshotWithSubDirInput() throws Exception {
+ // Test that:
+ // $ ozone fs -createSnapshot ofs://om/vol1/buck2/dir3/ snap1
+ //
+ // should print:
+ // Created snapshot ofs://om/vol1/buck2/.snapshot/snap1
+ //
+ // rather than:
+ // Created snapshot ofs://om/vol1/buck2/dir3/.snapshot/snap1
+
+ String snapshotName = "snap-" + RandomStringUtils.randomNumeric(5);
+
+ String dirPath = BUCKET_PATH + "/dir1/";
+
+ int res = ToolRunner.run(shell, new String[] {
+ "-mkdir", "-p", dirPath});
+ Assertions.assertEquals(0, res);
+
+ try (GenericTestUtils.SystemOutCapturer capture =
+ new GenericTestUtils.SystemOutCapturer()) {
+ res = ToolRunner.run(shell, new String[] {
+ "-createSnapshot", dirPath, snapshotName});
+ // Asserts that create request succeeded
+ Assertions.assertEquals(0, res);
+
+ String expectedSnapshotPath = Paths.get(
+ BUCKET_PATH, OM_SNAPSHOT_INDICATOR, snapshotName).toString();
+ String out = capture.getOutput().trim();
+ Assertions.assertTrue(out.endsWith(expectedSnapshotPath));
+ }
+ }
+
Review Comment:
Good point. Done.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #5174: HDDS-9157. [Snapshot] Print correct snapshot path in `fs -createSnapshot`
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #5174:
URL: https://github.com/apache/ozone/pull/5174#discussion_r1290943684
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFsSnapshot.java:
##########
@@ -138,6 +139,39 @@ public void testCreateSnapshotDuplicateName() throws Exception {
Assertions.assertEquals(1, res);
}
+ @Test
+ public void testCreateSnapshotWithSubDirInput() throws Exception {
+ // Test that:
+ // $ ozone fs -createSnapshot ofs://om/vol1/buck2/dir3/ snap1
+ //
+ // should print:
+ // Created snapshot ofs://om/vol1/buck2/.snapshot/snap1
+ //
+ // rather than:
+ // Created snapshot ofs://om/vol1/buck2/dir3/.snapshot/snap1
+
+ String snapshotName = "snap-" + RandomStringUtils.randomNumeric(5);
+
+ String dirPath = BUCKET_PATH + "/dir1/";
+
+ int res = ToolRunner.run(shell, new String[] {
+ "-mkdir", "-p", dirPath});
+ Assertions.assertEquals(0, res);
+
+ try (GenericTestUtils.SystemOutCapturer capture =
+ new GenericTestUtils.SystemOutCapturer()) {
+ res = ToolRunner.run(shell, new String[] {
+ "-createSnapshot", dirPath, snapshotName});
+ // Asserts that create request succeeded
+ Assertions.assertEquals(0, res);
+
+ String expectedSnapshotPath = Paths.get(
+ BUCKET_PATH, OM_SNAPSHOT_INDICATOR, snapshotName).toString();
+ String out = capture.getOutput().trim();
+ Assertions.assertTrue(out.endsWith(expectedSnapshotPath));
+ }
+ }
+
Review Comment:
It would also be good to have a unit test in TestBasicOzoneFilesystem & TestBasicRootedFilesystem class as well.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on pull request #5174: HDDS-9157. [Snapshot] Print correct snapshot path in `fs -createSnapshot`
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #5174:
URL: https://github.com/apache/ozone/pull/5174#issuecomment-1688701592
Thanks @aswinshakil @swamirishi for reviewing this!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl merged pull request #5174: HDDS-9157. [Snapshot] Print correct snapshot path in `fs -createSnapshot`
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl merged PR #5174:
URL: https://github.com/apache/ozone/pull/5174
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #5174: HDDS-9157. [Snapshot] Print correct snapshot path in `fs -createSnapshot`
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #5174:
URL: https://github.com/apache/ozone/pull/5174#discussion_r1290945147
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFsSnapshot.java:
##########
@@ -138,6 +139,39 @@ public void testCreateSnapshotDuplicateName() throws Exception {
Assertions.assertEquals(1, res);
}
+ @Test
+ public void testCreateSnapshotWithSubDirInput() throws Exception {
+ // Test that:
+ // $ ozone fs -createSnapshot ofs://om/vol1/buck2/dir3/ snap1
+ //
+ // should print:
+ // Created snapshot ofs://om/vol1/buck2/.snapshot/snap1
+ //
+ // rather than:
+ // Created snapshot ofs://om/vol1/buck2/dir3/.snapshot/snap1
+
+ String snapshotName = "snap-" + RandomStringUtils.randomNumeric(5);
+
+ String dirPath = BUCKET_PATH + "/dir1/";
+
+ int res = ToolRunner.run(shell, new String[] {
+ "-mkdir", "-p", dirPath});
+ Assertions.assertEquals(0, res);
+
+ try (GenericTestUtils.SystemOutCapturer capture =
+ new GenericTestUtils.SystemOutCapturer()) {
+ res = ToolRunner.run(shell, new String[] {
+ "-createSnapshot", dirPath, snapshotName});
+ // Asserts that create request succeeded
+ Assertions.assertEquals(0, res);
+
+ String expectedSnapshotPath = Paths.get(
+ BUCKET_PATH, OM_SNAPSHOT_INDICATOR, snapshotName).toString();
+ String out = capture.getOutput().trim();
+ Assertions.assertTrue(out.endsWith(expectedSnapshotPath));
+ }
+ }
+
Review Comment:
In this test we are relying on log output. Would be better to test the function as well.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org