HADOOP-16658 - S3A connector does not support including the token ren…#1664
HADOOP-16658 - S3A connector does not support including the token ren…#1664pzampino wants to merge 1 commit intoapache:trunkfrom
Conversation
There was a problem hiding this comment.
Is it possible for renewer to not be null but actually be empty?
There was a problem hiding this comment.
It is possible for it to be empty, in which case the renewer Text object will be "empty", and that case is checked by yarn.
There was a problem hiding this comment.
- need to make sure that null results in renewer == new Text()
- add some spaces round the
!=
There was a problem hiding this comment.
As these are public methods, are you sure that we don't need to preserve the old signature as well for possible extensions? Does this appear in a public release yet at all?
There was a problem hiding this comment.
This has not yet appeared in a public release; Steve actually suggested that I remove the previously-existing signatures, which I had marked as deprecated initially.
There was a problem hiding this comment.
no ASF release. The only people using this outside the hadoop-aws jar are you; I have plans to do other incompatible changes soon. Sorry.
There was a problem hiding this comment.
add a message for better diags, e.g. "renewer"
...ws/src/test/java/org/apache/hadoop/fs/s3a/auth/delegation/TestS3ADelegationTokenSupport.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
add message "renewer in " + ids
There was a problem hiding this comment.
add message "renewer in " + ids
There was a problem hiding this comment.
message "renewer in " + ids
There was a problem hiding this comment.
the renewer may now be null. should we add a new Text() if a null comes in?
There was a problem hiding this comment.
add an assert in this test case to verify load of renewer
|
Code LGTM; some minor comments about tests I'd like AbstractS3ATokenIdentifier to create a Text() if a null renewer was passed in; this is consistent with the existing code. |
…ewer in the token identifier
pzampino
force-pushed
the
HADOOP-16658
branch
from
1ce34cf to
7badaf2
Compare
|
I've made the suggested changes, and re-ran the tests (which passed again). |
|
🎊 +1 overall
This message was automatically generated. |
|
LGTM; thanks for changes +1 and merged to trunk |
4 participants
…ewer in the token identifier
Ran 'mvn verify' with endpoint s3.us-east-1.amazonaws.com
Two tests failed, but passed when run individually:
NOTICE
Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HADOOP-XXXXX. Fix a typo in YYY.)
For more details, please see https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute