-
Notifications
You must be signed in to change notification settings - Fork 616
HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well. #4042
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
8125de8
a11acd6
7e64e58
4045ed3
4b07e59
50d7eb5
b7d5a95
7d51838
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ | |
| import org.apache.hadoop.hdds.utils.db.TableIterator; | ||
| import org.apache.hadoop.ozone.om.helpers.BucketLayout; | ||
| import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; | ||
| import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo; | ||
| import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; | ||
| import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils; | ||
| import org.apache.hadoop.ozone.recon.api.types.DUResponse; | ||
|
|
@@ -322,4 +323,11 @@ public Table<String, OmKeyInfo> getKeyTable() { | |
| getOmMetadataManager().getKeyTable(getBucketLayout()); | ||
| return keyTable; | ||
| } | ||
|
|
||
| @Override | ||
| public OmDirectoryInfo getDirInfo(String[] names) throws IOException { | ||
| return OmDirectoryInfo.newBuilder() | ||
| .setName(names[2]) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If Add a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @smengcl - this check is not needed here as above code flow hits only when the path will be directory else not.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you explain more, what guarantees that we can't call this
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @devmadhuu I get that the only caller (at the moment) calling this method won't cause issues. I'm fine with either adding or not adding the check but I would prefer the former. Same goes for the method in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dombizita - getDirInfo method has caller only through DirectoryEntityHandler and call stack for DirectoryEntityHandler will be determined after validating the path , if path is directory, then only DirectoryEntityHandler will be returned by EntityHandler.getEntityHandler , so there is no way the current code flow will hit getDirInfo method if path is not directory. First two indexes will be taken by Volume and Bucket in path and 3rd index will be surely a directory. Please check the caller hierarchy of this method. We can have zoom call to explain by sharing my screen.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I get that currently this is only called when it is guaranteed that the path is a directory. but if someone later adds something and doesn't know how this method works the scenario can happen which @smengcl wrote above. all the classes that are extending the |
||
| .build(); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why aren't we asserting
getNumVolume()andgetNumBucket()here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @hemantk-12 for review. Changes done. Pls re-review. This was existing case, I added those missing as you mentioned.