-
Notifications
You must be signed in to change notification settings - Fork 9k
HDFS-17043. HttpFS implementation for getAllErasureCodingPolicies #5734
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
Conversation
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Map<String, String> params = new HashMap<>(); | ||
params.put(OP_PARAM, Operation.GETECPOLICIES.toString()); | ||
HttpURLConnection conn = | ||
getConnection(Operation.GETECPOLICIES.getMethod(), params, new Path(getUri() |
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.
expend, getUri().toString()
uses variables, which may make the code clearer
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 for your suggestion, I'll make the required code changes promptly
LGTM. |
🎊 +1 overall
This message was automatically generated. |
@ayushtkn Would you mind to take another reviews? Thanks. |
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.
dropped minor comments else lgtm
params.put(OP_PARAM, Operation.GETECPOLICIES.toString()); | ||
Path path = new Path(getUri().toString(), "/"); | ||
HttpURLConnection conn = | ||
getConnection(Operation.GETECPOLICIES.getMethod(), params, path, true); |
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.
do we need this makeQualified to be true
here? we hardcoded / itself here, getAllStoragePolicies seems to pass false
as well
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.
Thank you for your valuable suggestion. I sincerely appreciate it and will promptly implement the required adjustments to the code!
WebHdfsFileSystem webHdfsFileSystem = (WebHdfsFileSystem) httpFs; | ||
diffErasureCodingPolicies = webHdfsFileSystem.getAllErasureCodingPolicies(); | ||
} else { | ||
Assert.fail(fs.getClass().getSimpleName() + " doesn't support getSnapshotDiff"); |
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.
getSnapshotDiff
?
🎊 +1 overall
This message was automatically generated. |
@ayushtkn @slfan1989 Thank you for your assistance in reviewing the code! |
JIRA: HDFS-17043. HttpFS implementation for getAllErasureCodingPolicies