You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/02/12 02:45:28 UTC
[GitHub] [hadoop-ozone] smengcl opened a new pull request #547: HDDS-2928.
Implement ofs://: listStatus
smengcl opened a new pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547
## What changes were proposed in this pull request?
ofs:// should be able to handle list (recursive or not) under "root" and under each volume "directory", as if it is a flat filesystem (if the user has permissions to see the volumes).
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-2928
## How was this patch tested?
So far manually tested in CLI. See commit message for instructions.
Adding unit tests.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #547:
HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382697120
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -664,17 +732,73 @@ private FileStatusAdapter toFileStatusAdapter(OzoneFileStatus status) {
);
}
+ /**
+ * Generate a FileStatusAdapter for a volume.
+ * @param ozoneVolume OzoneVolume object
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a volume.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForVolume(
+ OzoneVolume ozoneVolume, URI uri) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneVolume.getName();
+ LOG.debug("getFileStatusAdapterForVolume(pathStr=" + pathStr);
+ Path path = new Path(pathStr);
+ return new FileStatusAdapter(
+ 0L,
+ path,
+ true,
+ (short)0,
+ 0L,
+ ozoneVolume.getCreationTime().getEpochSecond() * 1000,
+ 0L,
+ (short)00755, // Default directory permission, derive from ACLs later?
+ ozoneVolume.getOwner(),
+ ozoneVolume.getAdmin(), // TODO: Get group of whom?
+ path
+ );
+ }
+
+ /**
+ * Generate a FileStatusAdapter for a bucket.
+ * @param ozoneBucket OzoneBucket object.
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a bucket.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForBucket(
+ OzoneBucket ozoneBucket, URI uri, String username) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneBucket.getVolumeName() +
+ OZONE_URI_DELIMITER + ozoneBucket.getName();
+ LOG.debug("getFileStatusAdapterForBucket(pathStr=" + pathStr);
Review comment:
done in 936f87472fcb204128b3ab53a8afd978d288076c
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#547: HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382217294
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -439,11 +440,15 @@ public FileStatusAdapter getFileStatus(String path, URI uri,
incrementCounter(Statistic.OBJECTS_QUERY);
OFSPath ofsPath = new OFSPath(path);
String key = ofsPath.getKeyName();
- // getFileStatus is called for root
+ // path is root
if (ofsPath.getVolumeName().isEmpty() &&
ofsPath.getBucketName().isEmpty()) {
Review comment:
NIT: Can we wrap the logic into OFSPath#isRoot?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#547: HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382219238
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -664,17 +732,73 @@ private FileStatusAdapter toFileStatusAdapter(OzoneFileStatus status) {
);
}
+ /**
+ * Generate a FileStatusAdapter for a volume.
+ * @param ozoneVolume OzoneVolume object
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a volume.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForVolume(
+ OzoneVolume ozoneVolume, URI uri) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneVolume.getName();
+ LOG.debug("getFileStatusAdapterForVolume(pathStr=" + pathStr);
+ Path path = new Path(pathStr);
+ return new FileStatusAdapter(
+ 0L,
+ path,
+ true,
+ (short)0,
+ 0L,
+ ozoneVolume.getCreationTime().getEpochSecond() * 1000,
+ 0L,
+ (short)00755, // Default directory permission, derive from ACLs later?
+ ozoneVolume.getOwner(),
+ ozoneVolume.getAdmin(), // TODO: Get group of whom?
+ path
+ );
+ }
+
+ /**
+ * Generate a FileStatusAdapter for a bucket.
+ * @param ozoneBucket OzoneBucket object.
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a bucket.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForBucket(
+ OzoneBucket ozoneBucket, URI uri, String username) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneBucket.getVolumeName() +
+ OZONE_URI_DELIMITER + ozoneBucket.getName();
+ LOG.debug("getFileStatusAdapterForBucket(pathStr=" + pathStr);
+ Path path = new Path(pathStr);
+ return new FileStatusAdapter(
+ 0L,
+ path,
+ true,
+ (short)0,
+ 0L,
+ ozoneBucket.getCreationTime().getEpochSecond() * 1000,
+ 0L,
+ (short)00755, // Default directory permission, derive from ACLs later?
+ username, // TODO: owner and group.
+ username,
+ path
+ );
+ }
+
/**
* Generate a FileStatusAdapter for OFS root.
+ * @param uri Full URI to OFS root.
* @return FileStatusAdapter for root.
*/
- private static FileStatusAdapter rootFileStatusAdapter() {
+ private static FileStatusAdapter getFileStatusAdapterForRoot(URI uri) {
// Note that most fields are mimicked from HDFS FileStatus for root,
// except modification time, permission, owner and group.
- // TODO: Revisit the return value.
+ Path path = new Path(uri.toString() + OZONE_URI_DELIMITER);
return new FileStatusAdapter(
Review comment:
Can we have a single static RootFileStatusAdapter for all?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #547:
HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382687859
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -664,17 +732,73 @@ private FileStatusAdapter toFileStatusAdapter(OzoneFileStatus status) {
);
}
+ /**
+ * Generate a FileStatusAdapter for a volume.
+ * @param ozoneVolume OzoneVolume object
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a volume.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForVolume(
+ OzoneVolume ozoneVolume, URI uri) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneVolume.getName();
+ LOG.debug("getFileStatusAdapterForVolume(pathStr=" + pathStr);
+ Path path = new Path(pathStr);
+ return new FileStatusAdapter(
+ 0L,
+ path,
+ true,
+ (short)0,
+ 0L,
+ ozoneVolume.getCreationTime().getEpochSecond() * 1000,
+ 0L,
+ (short)00755, // Default directory permission, derive from ACLs later?
+ ozoneVolume.getOwner(),
+ ozoneVolume.getAdmin(), // TODO: Get group of whom?
+ path
+ );
+ }
+
+ /**
+ * Generate a FileStatusAdapter for a bucket.
+ * @param ozoneBucket OzoneBucket object.
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a bucket.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForBucket(
+ OzoneBucket ozoneBucket, URI uri, String username) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneBucket.getVolumeName() +
+ OZONE_URI_DELIMITER + ozoneBucket.getName();
+ LOG.debug("getFileStatusAdapterForBucket(pathStr=" + pathStr);
+ Path path = new Path(pathStr);
+ return new FileStatusAdapter(
+ 0L,
+ path,
+ true,
+ (short)0,
+ 0L,
+ ozoneBucket.getCreationTime().getEpochSecond() * 1000,
+ 0L,
+ (short)00755, // Default directory permission, derive from ACLs later?
+ username, // TODO: owner and group.
+ username,
+ path
+ );
+ }
+
/**
* Generate a FileStatusAdapter for OFS root.
+ * @param uri Full URI to OFS root.
* @return FileStatusAdapter for root.
*/
- private static FileStatusAdapter rootFileStatusAdapter() {
+ private static FileStatusAdapter getFileStatusAdapterForRoot(URI uri) {
// Note that most fields are mimicked from HDFS FileStatus for root,
// except modification time, permission, owner and group.
- // TODO: Revisit the return value.
+ Path path = new Path(uri.toString() + OZONE_URI_DELIMITER);
return new FileStatusAdapter(
Review comment:
The URI part (`ofs://localhost:54607`) can differ each time the client application initializes FileSystem. So I'd say we can't have a static `RootFileStatusAdapter`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #547:
HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382699982
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -511,10 +567,22 @@ public void makeQualified(FileStatus status, URI uri, Path path,
incrementCounter(Statistic.OBJECTS_LIST);
OFSPath ofsPath = new OFSPath(pathStr);
- // TODO: Subject to change in HDDS-2928.
+
+ // listStatus root
+ if (ofsPath.getVolumeName().isEmpty()
Review comment:
done in ada1ac4b0b0164c71e912a70c495c269887cde09
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #547:
HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382692666
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -664,17 +732,73 @@ private FileStatusAdapter toFileStatusAdapter(OzoneFileStatus status) {
);
}
+ /**
+ * Generate a FileStatusAdapter for a volume.
+ * @param ozoneVolume OzoneVolume object
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a volume.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForVolume(
+ OzoneVolume ozoneVolume, URI uri) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneVolume.getName();
+ LOG.debug("getFileStatusAdapterForVolume(pathStr=" + pathStr);
+ Path path = new Path(pathStr);
+ return new FileStatusAdapter(
+ 0L,
+ path,
+ true,
+ (short)0,
+ 0L,
+ ozoneVolume.getCreationTime().getEpochSecond() * 1000,
+ 0L,
+ (short)00755, // Default directory permission, derive from ACLs later?
Review comment:
Done in b5cc3e4035df0267b60354224980d3a4da1f89e4
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #547:
HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382684259
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -439,11 +440,15 @@ public FileStatusAdapter getFileStatus(String path, URI uri,
incrementCounter(Statistic.OBJECTS_QUERY);
OFSPath ofsPath = new OFSPath(path);
String key = ofsPath.getKeyName();
- // getFileStatus is called for root
+ // path is root
if (ofsPath.getVolumeName().isEmpty() &&
ofsPath.getBucketName().isEmpty()) {
Review comment:
Done in f003b87c307b5b3aae91a5fe25d3b81bc945ec3d
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#547: HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382220405
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -664,17 +732,73 @@ private FileStatusAdapter toFileStatusAdapter(OzoneFileStatus status) {
);
}
+ /**
+ * Generate a FileStatusAdapter for a volume.
+ * @param ozoneVolume OzoneVolume object
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a volume.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForVolume(
+ OzoneVolume ozoneVolume, URI uri) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneVolume.getName();
+ LOG.debug("getFileStatusAdapterForVolume(pathStr=" + pathStr);
+ Path path = new Path(pathStr);
+ return new FileStatusAdapter(
+ 0L,
+ path,
+ true,
+ (short)0,
+ 0L,
+ ozoneVolume.getCreationTime().getEpochSecond() * 1000,
+ 0L,
+ (short)00755, // Default directory permission, derive from ACLs later?
+ ozoneVolume.getOwner(),
+ ozoneVolume.getAdmin(), // TODO: Get group of whom?
+ path
+ );
+ }
+
+ /**
+ * Generate a FileStatusAdapter for a bucket.
+ * @param ozoneBucket OzoneBucket object.
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a bucket.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForBucket(
+ OzoneBucket ozoneBucket, URI uri, String username) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneBucket.getVolumeName() +
+ OZONE_URI_DELIMITER + ozoneBucket.getName();
+ LOG.debug("getFileStatusAdapterForBucket(pathStr=" + pathStr);
Review comment:
Let do this with
if (LOG.isDebugEnabled()) {
LOG.debug("getFileStatusAdapterForBucket(pathStr={}", pathStr);
}
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #547:
HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382711568
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -486,6 +491,57 @@ public void makeQualified(FileStatus status, URI uri, Path path,
return new IteratorAdapter(bucket.listKeys(key));
}
+ /**
+ * Helper for OFS listStatus on root.
+ */
+ private List<FileStatusAdapter> listStatusRoot(
+ boolean recursive, String startPath, long numEntries,
+ URI uri, Path workingDir, String username) throws IOException {
+ OFSPath ofsStartPath = new OFSPath(startPath);
+ // list volumes for user
+ Iterator<? extends OzoneVolume> iter = objectStore.listVolumesByUser(
+ username, null, ofsStartPath.getVolumeName());
+ List<FileStatusAdapter> res = new ArrayList<>();
+ // TODO: Test continuation
+ while (iter.hasNext() && res.size() <= numEntries) {
+ OzoneVolume volume = iter.next();
+ res.add(getFileStatusAdapterForVolume(volume, uri));
+ if (recursive) {
+ String next = volume.getName();
+ // TODO: Check startPath
+ res.addAll(listStatus(next, recursive, startPath,
+ numEntries - res.size(), uri, workingDir, username));
+ }
+ }
+ return res;
+ }
+
+ /**
+ * Helper for OFS listStatus on a volume.
+ */
+ private List<FileStatusAdapter> listStatusVolume(String volumeStr,
+ boolean recursive, String startPath, long numEntries,
+ URI uri, Path workingDir, String username) throws IOException {
+ OFSPath ofsStartPath = new OFSPath(startPath);
Review comment:
> I think we can implement this with either next or startPath but not necessarily both.
~~I just realized that the current implementation with both continuation (startPath) and recursion might not work as expected.~~
~~e.g. if such call is made: `listStatus(pathStr="/", recursive=true, startPath="/volume2/bucket2/dir2")`, the result won't be what we expect.~~
Actually, as I think again. It should just work. I just need to change `listStatusVolume` back to `startPath`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #547:
HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382711568
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -486,6 +491,57 @@ public void makeQualified(FileStatus status, URI uri, Path path,
return new IteratorAdapter(bucket.listKeys(key));
}
+ /**
+ * Helper for OFS listStatus on root.
+ */
+ private List<FileStatusAdapter> listStatusRoot(
+ boolean recursive, String startPath, long numEntries,
+ URI uri, Path workingDir, String username) throws IOException {
+ OFSPath ofsStartPath = new OFSPath(startPath);
+ // list volumes for user
+ Iterator<? extends OzoneVolume> iter = objectStore.listVolumesByUser(
+ username, null, ofsStartPath.getVolumeName());
+ List<FileStatusAdapter> res = new ArrayList<>();
+ // TODO: Test continuation
+ while (iter.hasNext() && res.size() <= numEntries) {
+ OzoneVolume volume = iter.next();
+ res.add(getFileStatusAdapterForVolume(volume, uri));
+ if (recursive) {
+ String next = volume.getName();
+ // TODO: Check startPath
+ res.addAll(listStatus(next, recursive, startPath,
+ numEntries - res.size(), uri, workingDir, username));
+ }
+ }
+ return res;
+ }
+
+ /**
+ * Helper for OFS listStatus on a volume.
+ */
+ private List<FileStatusAdapter> listStatusVolume(String volumeStr,
+ boolean recursive, String startPath, long numEntries,
+ URI uri, Path workingDir, String username) throws IOException {
+ OFSPath ofsStartPath = new OFSPath(startPath);
Review comment:
> I think we can implement this with either next or startPath but not necessarily both.
~~I just realized that the current implementation with both continuation (startPath) and recursion might not work as expected.~~
~~e.g. if such call is made: `listStatus(pathStr="/", recursive=true, startPath="/volume2/bucket2/dir2")`, the result won't be what we expect.~~
Actually, as I think again. It should just work. I just need to change `listStatusVolume` back to `startPath`. done in f94b95c4148d19dd00f2c16781b9d4a33be4b891
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#547: HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r383551878
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -664,17 +732,73 @@ private FileStatusAdapter toFileStatusAdapter(OzoneFileStatus status) {
);
}
+ /**
+ * Generate a FileStatusAdapter for a volume.
+ * @param ozoneVolume OzoneVolume object
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a volume.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForVolume(
+ OzoneVolume ozoneVolume, URI uri) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneVolume.getName();
+ LOG.debug("getFileStatusAdapterForVolume(pathStr=" + pathStr);
+ Path path = new Path(pathStr);
+ return new FileStatusAdapter(
+ 0L,
+ path,
+ true,
+ (short)0,
+ 0L,
+ ozoneVolume.getCreationTime().getEpochSecond() * 1000,
+ 0L,
+ (short)00755, // Default directory permission, derive from ACLs later?
+ ozoneVolume.getOwner(),
+ ozoneVolume.getAdmin(), // TODO: Get group of whom?
+ path
+ );
+ }
+
+ /**
+ * Generate a FileStatusAdapter for a bucket.
+ * @param ozoneBucket OzoneBucket object.
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a bucket.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForBucket(
+ OzoneBucket ozoneBucket, URI uri, String username) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneBucket.getVolumeName() +
+ OZONE_URI_DELIMITER + ozoneBucket.getName();
+ LOG.debug("getFileStatusAdapterForBucket(pathStr=" + pathStr);
+ Path path = new Path(pathStr);
+ return new FileStatusAdapter(
+ 0L,
+ path,
+ true,
+ (short)0,
+ 0L,
+ ozoneBucket.getCreationTime().getEpochSecond() * 1000,
+ 0L,
+ (short)00755, // Default directory permission, derive from ACLs later?
+ username, // TODO: owner and group.
+ username,
+ path
+ );
+ }
+
/**
* Generate a FileStatusAdapter for OFS root.
+ * @param uri Full URI to OFS root.
* @return FileStatusAdapter for root.
*/
- private static FileStatusAdapter rootFileStatusAdapter() {
+ private static FileStatusAdapter getFileStatusAdapterForRoot(URI uri) {
// Note that most fields are mimicked from HDFS FileStatus for root,
// except modification time, permission, owner and group.
- // TODO: Revisit the return value.
+ Path path = new Path(uri.toString() + OZONE_URI_DELIMITER);
return new FileStatusAdapter(
Review comment:
Agree.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#547: HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382227634
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -511,10 +567,22 @@ public void makeQualified(FileStatus status, URI uri, Path path,
incrementCounter(Statistic.OBJECTS_LIST);
OFSPath ofsPath = new OFSPath(pathStr);
- // TODO: Subject to change in HDDS-2928.
+
+ // listStatus root
+ if (ofsPath.getVolumeName().isEmpty()
Review comment:
Can we add OFSPath helpers as suggested 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#547: HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382216403
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -492,4 +493,42 @@ private void assertKeyNotFoundException(IOException ex) {
GenericTestUtils.assertExceptionContains("KEY_NOT_FOUND", ex);
}
+ /**
+ * OFS: Test recursive listStatus on root.
+ */
+ @Test
+ public void testListStatusRootAndVolumeNonRecursive() throws Exception {
Review comment:
Can we add a test case to cover recursive list status?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #547:
HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382687859
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -664,17 +732,73 @@ private FileStatusAdapter toFileStatusAdapter(OzoneFileStatus status) {
);
}
+ /**
+ * Generate a FileStatusAdapter for a volume.
+ * @param ozoneVolume OzoneVolume object
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a volume.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForVolume(
+ OzoneVolume ozoneVolume, URI uri) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneVolume.getName();
+ LOG.debug("getFileStatusAdapterForVolume(pathStr=" + pathStr);
+ Path path = new Path(pathStr);
+ return new FileStatusAdapter(
+ 0L,
+ path,
+ true,
+ (short)0,
+ 0L,
+ ozoneVolume.getCreationTime().getEpochSecond() * 1000,
+ 0L,
+ (short)00755, // Default directory permission, derive from ACLs later?
+ ozoneVolume.getOwner(),
+ ozoneVolume.getAdmin(), // TODO: Get group of whom?
+ path
+ );
+ }
+
+ /**
+ * Generate a FileStatusAdapter for a bucket.
+ * @param ozoneBucket OzoneBucket object.
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a bucket.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForBucket(
+ OzoneBucket ozoneBucket, URI uri, String username) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneBucket.getVolumeName() +
+ OZONE_URI_DELIMITER + ozoneBucket.getName();
+ LOG.debug("getFileStatusAdapterForBucket(pathStr=" + pathStr);
+ Path path = new Path(pathStr);
+ return new FileStatusAdapter(
+ 0L,
+ path,
+ true,
+ (short)0,
+ 0L,
+ ozoneBucket.getCreationTime().getEpochSecond() * 1000,
+ 0L,
+ (short)00755, // Default directory permission, derive from ACLs later?
+ username, // TODO: owner and group.
+ username,
+ path
+ );
+ }
+
/**
* Generate a FileStatusAdapter for OFS root.
+ * @param uri Full URI to OFS root.
* @return FileStatusAdapter for root.
*/
- private static FileStatusAdapter rootFileStatusAdapter() {
+ private static FileStatusAdapter getFileStatusAdapterForRoot(URI uri) {
// Note that most fields are mimicked from HDFS FileStatus for root,
// except modification time, permission, owner and group.
- // TODO: Revisit the return value.
+ Path path = new Path(uri.toString() + OZONE_URI_DELIMITER);
return new FileStatusAdapter(
Review comment:
~~The URI part (`ofs://localhost:54607`) can differ each time the client application initializes FileSystem. So I'd say we can't have a static `RootFileStatusAdapter` with the current impl.~~
Actually, the `uri` can be different each time `getFileStatus` is called, so we can't even have a class global variable to hold `rootFileStatusAdapter`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#547: HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382219605
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -664,17 +732,73 @@ private FileStatusAdapter toFileStatusAdapter(OzoneFileStatus status) {
);
}
+ /**
+ * Generate a FileStatusAdapter for a volume.
+ * @param ozoneVolume OzoneVolume object
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a volume.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForVolume(
+ OzoneVolume ozoneVolume, URI uri) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneVolume.getName();
+ LOG.debug("getFileStatusAdapterForVolume(pathStr=" + pathStr);
+ Path path = new Path(pathStr);
+ return new FileStatusAdapter(
+ 0L,
+ path,
+ true,
+ (short)0,
+ 0L,
+ ozoneVolume.getCreationTime().getEpochSecond() * 1000,
+ 0L,
+ (short)00755, // Default directory permission, derive from ACLs later?
Review comment:
NIT: can we define as a const?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #547:
HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382678624
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -492,4 +493,42 @@ private void assertKeyNotFoundException(IOException ex) {
GenericTestUtils.assertExceptionContains("KEY_NOT_FOUND", ex);
}
+ /**
+ * OFS: Test recursive listStatus on root.
+ */
+ @Test
+ public void testListStatusRootAndVolumeNonRecursive() throws Exception {
Review comment:
Done in 5048760863a73ae3712bed1f6b8b21f6ced247a8
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#547: HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382221623
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -664,17 +732,73 @@ private FileStatusAdapter toFileStatusAdapter(OzoneFileStatus status) {
);
}
+ /**
+ * Generate a FileStatusAdapter for a volume.
+ * @param ozoneVolume OzoneVolume object
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a volume.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForVolume(
+ OzoneVolume ozoneVolume, URI uri) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneVolume.getName();
+ LOG.debug("getFileStatusAdapterForVolume(pathStr=" + pathStr);
Review comment:
similar to the one below, please add conditional debug output with parameterized log.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #547:
HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382711568
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -486,6 +491,57 @@ public void makeQualified(FileStatus status, URI uri, Path path,
return new IteratorAdapter(bucket.listKeys(key));
}
+ /**
+ * Helper for OFS listStatus on root.
+ */
+ private List<FileStatusAdapter> listStatusRoot(
+ boolean recursive, String startPath, long numEntries,
+ URI uri, Path workingDir, String username) throws IOException {
+ OFSPath ofsStartPath = new OFSPath(startPath);
+ // list volumes for user
+ Iterator<? extends OzoneVolume> iter = objectStore.listVolumesByUser(
+ username, null, ofsStartPath.getVolumeName());
+ List<FileStatusAdapter> res = new ArrayList<>();
+ // TODO: Test continuation
+ while (iter.hasNext() && res.size() <= numEntries) {
+ OzoneVolume volume = iter.next();
+ res.add(getFileStatusAdapterForVolume(volume, uri));
+ if (recursive) {
+ String next = volume.getName();
+ // TODO: Check startPath
+ res.addAll(listStatus(next, recursive, startPath,
+ numEntries - res.size(), uri, workingDir, username));
+ }
+ }
+ return res;
+ }
+
+ /**
+ * Helper for OFS listStatus on a volume.
+ */
+ private List<FileStatusAdapter> listStatusVolume(String volumeStr,
+ boolean recursive, String startPath, long numEntries,
+ URI uri, Path workingDir, String username) throws IOException {
+ OFSPath ofsStartPath = new OFSPath(startPath);
Review comment:
> I think we can implement this with either next or startPath but not necessarily both.
I just realized that the current implementation with both continuation (startPath) and recursion might not work as expected.
e.g. if such call is made: `listStatus(pathStr="/", recursive=true, startPath="/volume2/bucket2/dir2")`, the result won't be what we expect.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao merged pull request #547: HDDS-2928.
Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
xiaoyuyao merged pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #547:
HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382684363
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -439,11 +440,15 @@ public FileStatusAdapter getFileStatus(String path, URI uri,
incrementCounter(Statistic.OBJECTS_QUERY);
OFSPath ofsPath = new OFSPath(path);
String key = ofsPath.getKeyName();
- // getFileStatus is called for root
+ // path is root
if (ofsPath.getVolumeName().isEmpty() &&
ofsPath.getBucketName().isEmpty()) {
- // Generate a FileStatusAdapter for root
- return rootFileStatusAdapter();
+ return getFileStatusAdapterForRoot(uri);
+ }
+ // path is a volume
+ if (ofsPath.getBucketName().isEmpty()) {
Review comment:
Done in f003b87c307b5b3aae91a5fe25d3b81bc945ec3d
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #547:
HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382263745
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -492,4 +493,42 @@ private void assertKeyNotFoundException(IOException ex) {
GenericTestUtils.assertExceptionContains("KEY_NOT_FOUND", ex);
}
+ /**
+ * OFS: Test recursive listStatus on root.
+ */
+ @Test
+ public void testListStatusRootAndVolumeNonRecursive() throws Exception {
+ String volume1 = getRandomNonExistVolumeName();
Review comment:
done in ab03db64f18a75fdd5fcc0032ae7170e942576bf
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#547: HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382215784
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -492,4 +493,42 @@ private void assertKeyNotFoundException(IOException ex) {
GenericTestUtils.assertExceptionContains("KEY_NOT_FOUND", ex);
}
+ /**
+ * OFS: Test recursive listStatus on root.
+ */
+ @Test
+ public void testListStatusRootAndVolumeNonRecursive() throws Exception {
+ String volume1 = getRandomNonExistVolumeName();
Review comment:
NIT: can we refactor to dedup line 501-517.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #547:
HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382687859
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -664,17 +732,73 @@ private FileStatusAdapter toFileStatusAdapter(OzoneFileStatus status) {
);
}
+ /**
+ * Generate a FileStatusAdapter for a volume.
+ * @param ozoneVolume OzoneVolume object
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a volume.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForVolume(
+ OzoneVolume ozoneVolume, URI uri) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneVolume.getName();
+ LOG.debug("getFileStatusAdapterForVolume(pathStr=" + pathStr);
+ Path path = new Path(pathStr);
+ return new FileStatusAdapter(
+ 0L,
+ path,
+ true,
+ (short)0,
+ 0L,
+ ozoneVolume.getCreationTime().getEpochSecond() * 1000,
+ 0L,
+ (short)00755, // Default directory permission, derive from ACLs later?
+ ozoneVolume.getOwner(),
+ ozoneVolume.getAdmin(), // TODO: Get group of whom?
+ path
+ );
+ }
+
+ /**
+ * Generate a FileStatusAdapter for a bucket.
+ * @param ozoneBucket OzoneBucket object.
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a bucket.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForBucket(
+ OzoneBucket ozoneBucket, URI uri, String username) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneBucket.getVolumeName() +
+ OZONE_URI_DELIMITER + ozoneBucket.getName();
+ LOG.debug("getFileStatusAdapterForBucket(pathStr=" + pathStr);
+ Path path = new Path(pathStr);
+ return new FileStatusAdapter(
+ 0L,
+ path,
+ true,
+ (short)0,
+ 0L,
+ ozoneBucket.getCreationTime().getEpochSecond() * 1000,
+ 0L,
+ (short)00755, // Default directory permission, derive from ACLs later?
+ username, // TODO: owner and group.
+ username,
+ path
+ );
+ }
+
/**
* Generate a FileStatusAdapter for OFS root.
+ * @param uri Full URI to OFS root.
* @return FileStatusAdapter for root.
*/
- private static FileStatusAdapter rootFileStatusAdapter() {
+ private static FileStatusAdapter getFileStatusAdapterForRoot(URI uri) {
// Note that most fields are mimicked from HDFS FileStatus for root,
// except modification time, permission, owner and group.
- // TODO: Revisit the return value.
+ Path path = new Path(uri.toString() + OZONE_URI_DELIMITER);
return new FileStatusAdapter(
Review comment:
The URI part (`ofs://localhost:54607`) can differ each time the client application initializes FileSystem. So I'd say we can't have a static `RootFileStatusAdapter` with the current impl.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #547:
HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382698808
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -664,17 +732,73 @@ private FileStatusAdapter toFileStatusAdapter(OzoneFileStatus status) {
);
}
+ /**
+ * Generate a FileStatusAdapter for a volume.
+ * @param ozoneVolume OzoneVolume object
+ * @param uri Full URI to OFS root.
+ * @return FileStatusAdapter for a volume.
+ */
+ private static FileStatusAdapter getFileStatusAdapterForVolume(
+ OzoneVolume ozoneVolume, URI uri) {
+ String pathStr = uri.toString() +
+ OZONE_URI_DELIMITER + ozoneVolume.getName();
+ LOG.debug("getFileStatusAdapterForVolume(pathStr=" + pathStr);
Review comment:
done in 3553f6301a70292c4643fe8ded21e91d718334e8
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #547:
HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382703266
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -486,6 +491,57 @@ public void makeQualified(FileStatus status, URI uri, Path path,
return new IteratorAdapter(bucket.listKeys(key));
}
+ /**
+ * Helper for OFS listStatus on root.
+ */
+ private List<FileStatusAdapter> listStatusRoot(
+ boolean recursive, String startPath, long numEntries,
+ URI uri, Path workingDir, String username) throws IOException {
+ OFSPath ofsStartPath = new OFSPath(startPath);
+ // list volumes for user
+ Iterator<? extends OzoneVolume> iter = objectStore.listVolumesByUser(
+ username, null, ofsStartPath.getVolumeName());
+ List<FileStatusAdapter> res = new ArrayList<>();
+ // TODO: Test continuation
+ while (iter.hasNext() && res.size() <= numEntries) {
+ OzoneVolume volume = iter.next();
+ res.add(getFileStatusAdapterForVolume(volume, uri));
+ if (recursive) {
+ String next = volume.getName();
+ // TODO: Check startPath
+ res.addAll(listStatus(next, recursive, startPath,
+ numEntries - res.size(), uri, workingDir, username));
+ }
+ }
+ return res;
+ }
+
+ /**
+ * Helper for OFS listStatus on a volume.
+ */
+ private List<FileStatusAdapter> listStatusVolume(String volumeStr,
+ boolean recursive, String startPath, long numEntries,
+ URI uri, Path workingDir, String username) throws IOException {
+ OFSPath ofsStartPath = new OFSPath(startPath);
Review comment:
> should we use startBucket as parameter to avoid creating OFSPath unnecessarily?
Done in 41c081f2f6e506f0942fe89ad5ced91df6e2c3d0
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#547: HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382228732
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -486,6 +491,57 @@ public void makeQualified(FileStatus status, URI uri, Path path,
return new IteratorAdapter(bucket.listKeys(key));
}
+ /**
+ * Helper for OFS listStatus on root.
+ */
+ private List<FileStatusAdapter> listStatusRoot(
+ boolean recursive, String startPath, long numEntries,
+ URI uri, Path workingDir, String username) throws IOException {
+ OFSPath ofsStartPath = new OFSPath(startPath);
+ // list volumes for user
+ Iterator<? extends OzoneVolume> iter = objectStore.listVolumesByUser(
+ username, null, ofsStartPath.getVolumeName());
+ List<FileStatusAdapter> res = new ArrayList<>();
+ // TODO: Test continuation
+ while (iter.hasNext() && res.size() <= numEntries) {
+ OzoneVolume volume = iter.next();
+ res.add(getFileStatusAdapterForVolume(volume, uri));
+ if (recursive) {
+ String next = volume.getName();
+ // TODO: Check startPath
+ res.addAll(listStatus(next, recursive, startPath,
+ numEntries - res.size(), uri, workingDir, username));
+ }
+ }
+ return res;
+ }
+
+ /**
+ * Helper for OFS listStatus on a volume.
+ */
+ private List<FileStatusAdapter> listStatusVolume(String volumeStr,
+ boolean recursive, String startPath, long numEntries,
+ URI uri, Path workingDir, String username) throws IOException {
+ OFSPath ofsStartPath = new OFSPath(startPath);
Review comment:
I think we can implement this with either next or startPath but not necessarily both.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #547:
HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382248299
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -439,11 +440,15 @@ public FileStatusAdapter getFileStatus(String path, URI uri,
incrementCounter(Statistic.OBJECTS_QUERY);
OFSPath ofsPath = new OFSPath(path);
String key = ofsPath.getKeyName();
- // getFileStatus is called for root
+ // path is root
if (ofsPath.getVolumeName().isEmpty() &&
ofsPath.getBucketName().isEmpty()) {
Review comment:
Good idea. Cleaner that way.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#547: HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382217631
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -439,11 +440,15 @@ public FileStatusAdapter getFileStatus(String path, URI uri,
incrementCounter(Statistic.OBJECTS_QUERY);
OFSPath ofsPath = new OFSPath(path);
String key = ofsPath.getKeyName();
- // getFileStatus is called for root
+ // path is root
if (ofsPath.getVolumeName().isEmpty() &&
ofsPath.getBucketName().isEmpty()) {
- // Generate a FileStatusAdapter for root
- return rootFileStatusAdapter();
+ return getFileStatusAdapterForRoot(uri);
+ }
+ // path is a volume
+ if (ofsPath.getBucketName().isEmpty()) {
Review comment:
Same as above. Can we wrap with OFSPath#isVolume?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#547: HDDS-2928. Implement ofs://: listStatus
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #547: HDDS-2928. Implement ofs://: listStatus
URL: https://github.com/apache/hadoop-ozone/pull/547#discussion_r382227426
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -486,6 +491,57 @@ public void makeQualified(FileStatus status, URI uri, Path path,
return new IteratorAdapter(bucket.listKeys(key));
}
+ /**
+ * Helper for OFS listStatus on root.
+ */
+ private List<FileStatusAdapter> listStatusRoot(
+ boolean recursive, String startPath, long numEntries,
+ URI uri, Path workingDir, String username) throws IOException {
+ OFSPath ofsStartPath = new OFSPath(startPath);
+ // list volumes for user
+ Iterator<? extends OzoneVolume> iter = objectStore.listVolumesByUser(
+ username, null, ofsStartPath.getVolumeName());
+ List<FileStatusAdapter> res = new ArrayList<>();
+ // TODO: Test continuation
+ while (iter.hasNext() && res.size() <= numEntries) {
+ OzoneVolume volume = iter.next();
+ res.add(getFileStatusAdapterForVolume(volume, uri));
+ if (recursive) {
+ String next = volume.getName();
+ // TODO: Check startPath
+ res.addAll(listStatus(next, recursive, startPath,
+ numEntries - res.size(), uri, workingDir, username));
+ }
+ }
+ return res;
+ }
+
+ /**
+ * Helper for OFS listStatus on a volume.
+ */
+ private List<FileStatusAdapter> listStatusVolume(String volumeStr,
+ boolean recursive, String startPath, long numEntries,
+ URI uri, Path workingDir, String username) throws IOException {
+ OFSPath ofsStartPath = new OFSPath(startPath);
Review comment:
should we use startBucket as parameter to avoid creating OFSPath unnecessarily?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org