[GitHub] [hadoop] liuml07 commented on pull request #2197: HADOOP-17159 Ability for forceful relogin in UserGroupInformation class
liuml07 commented on pull request #2197: URL: https://github.com/apache/hadoop/pull/2197#issuecomment-681026137 @sguggilam >and use false for both parameter? Yeah, the second parameter actually is `true`, but you got what I meant as the above code snippet. I have reverted from `trunk` branch, and will do that for other branches. Now you can file a new PR to include everything (updated). Thanks! 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: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] liuml07 commented on pull request #2197: HADOOP-17159 Ability for forceful relogin in UserGroupInformation class
liuml07 commented on pull request #2197:
URL: https://github.com/apache/hadoop/pull/2197#issuecomment-681017115
@sguggilam We both agree on that we need to change all two callsites in UGI.
For the new API, if we keep it as it, is a bit confusing when user calls with
`false` value when user does not need to ignore last login time. In that case,
we still ignore the TGT check since the first parameter is always `false`.
```
public void reloginFromKeytab(boolean ignoreTimeElapsed) throws
IOException {
reloginFromKeytab(false, ignoreTimeElapsed);
}
```
Since the forceful login is the main target, how about we create a new
method name, and use `false` for both parameter?
```
public void forceReloginFromKeytab() throws IOException {
reloginFromKeytab(false, true);
}
```
I think since this a bug, it's better not to create followup JIRAs but
instead create a new full pull request. So when people backport or check the
change for debugging, they do not need to jump different PRs to understand
what's happening as addendum. I will revert the commit in git repo, and the new
PR will be including the full code. How about that?
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] liuml07 commented on pull request #2197: HADOOP-17159 Ability for forceful relogin in UserGroupInformation class
liuml07 commented on pull request #2197:
URL: https://github.com/apache/hadoop/pull/2197#issuecomment-680714535
@sguggilam When I backport the code, I went through the code again. I think
I found one bug. Before I revert the change, I want to confirm with you.
The new public API
```
public void reloginFromKeytab(boolean ignoreTimeElapsed) throws
IOException {
reloginFromKeytab(false, ignoreTimeElapsed);
}
```
Should be something like:
```
public void reloginFromKeytab(boolean checkTGT) throws IOException {
reloginFromKeytab(checkTGT, true);
}
```
This way, all new code can call this new API `public void
reloginFromKeytab(boolean checkTGT)` to force login ignoring the last login
time, but need to provide if they want to check TGT. Meanwhile, the existing
calls of `reloginFromKeytab(false)` or `reloginFromKeytab(true)`, the parameter
value `false` or `true` was previously for `checkTGT`. But with this change,
they are for `ignoreTimeElapsed`. So the code calling the previously private
method is broken - I found two places.
Could you explain more? Thanks!
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] liuml07 commented on pull request #2197: HADOOP-17159 Ability for forceful relogin in UserGroupInformation class
liuml07 commented on pull request #2197: URL: https://github.com/apache/hadoop/pull/2197#issuecomment-674491150 The patch looks good to me overall. A question is: should we rename the word "force" with "ignoreTimeElapsed" to make it clear. If agreed on it includes updating the parameters name and Javadoc. Thoughts? @sguggilam I will give a final review next week. 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: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] liuml07 commented on pull request #2197: HADOOP-17159 Ability for forceful relogin in UserGroupInformation class
liuml07 commented on pull request #2197: URL: https://github.com/apache/hadoop/pull/2197#issuecomment-670640493 Thanks @steveloughran I added Arpit and kihwal as reviewers. 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: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
