-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HDFS-17215. RBF: fix some method annotations about @throws . #6136
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. |
390c083 to
ad60097
Compare
|
💔 -1 overall
This message was automatically generated. |
| * By the way, this latter case actually throws an AccessControlException, | ||
| * which happens to be a subclass of IOException. |
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.
this isn't required I think,
below it throws ACE directly as well
if (checkMountEntry && isMountEntry(path)) {
throw new AccessControlException(
"Permission denied: " + RouterRpcServer.getRemoteUser()
+ " is not allowed to change quota of " + path);
}
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.
emmm, What I'm trying to show is that this @throw doesn't match what the method actually throws, as shown in the following screenshot
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.
Correcting that throws to IOE is ok, this Bu the way line isn’t required
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.
Ok, that's a little more concise. I have updated the PR, please check it again.
…s an error, which is described in the @throws section.
ad60097 to
119e426
Compare
|
💔 -1 overall
This message was automatically generated. |
ayushtkn
left a comment
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.
LGTM
slfan1989
left a comment
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.
@ayushtkn Thanks for helping review the code! @xiaojunxiang2023 Can we check if a package has the same issue? If we have the same issue, can we fix it in this PR?
8588c55 to
082edef
Compare
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
I have checked all @throws comments in the rbf module,and fixed them in this PR. |
| */ | ||
| @SuppressWarnings("unchecked") | ||
| public void setProto(Message p) { | ||
| public void setProto(Message p) throws IllegalArgumentException { // unchecked exception |
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.
I think it is not necessary to add the comment // unchecked exception 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.
@xiaojunxiang2023 can you address this? we can commit post this
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.
I changed it myself, will merge if the build comes green
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |

jira:https://issues.apache.org/jira/browse/HDFS-17215
The setQuota method annotation of the Quota class has an error, which is described in the @throws section.