-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HADOOP-15409 #367
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
HADOOP-15409 #367
Conversation
change doesBucketExistV2 to verified the acl
|
厉害 |
|
There are no conflicts in this right now. |
|
I do want to merge this in, but we have a very strict policy for the object stores "Say which endpoint you ran the integration tests for that store against"? Jenkins can't do it, and while I'll do a full test run before I do the final merge, I don't want to be the person debugging the acutal patch. see Policy_for_submitting_patches_which_affect_the_hadoop-aws_module thanks |
|
@steveloughran Thanks your comment, I will append the test case . |
|
@steveloughran can I apply the credentials for test ? |
append the Test case
|
afraid you get to use your own credentials. I'll do a final testrun before committing, but during dev you are going to have to pay. The tests are designed to keep costs down to cents; all data is deleted, the big downloads are on Amazon's own data, and if you don't run the scale tests (-Pscale) there's not much bulk data IO going on at al... |
|
@steveloughran Thanks reply. |
| try { | ||
| fs.verifyBucketExists(); | ||
| } catch (IOException e) { | ||
| fail(e.getMessage()); |
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.
Given we actually call verifyBucketExists in the initialize() routine, it'll have already been tested in setup. I think we can get by without adding a new test 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.
Ok, I will update the test case.
|
@steveloughran I have removed the test case ,please reveiw and comment. |
|
hey, it's great you've signed your patches. Can you submit your public key to the central GPG Servers? |
|
Oddly enough, one of the stack traces in troubleshooting_s3a shows doesBucketExist doing the ACK check, The sdk may have switched a long time ago |
|
Next patch for this will give you a choice of v1 or v2, so that third-party endpoints will work. Versions other than 1 or 2 get told off and then continue |
|
@steveloughran the public key has imported. thanks. |
Author: Jacob Maes <[email protected]> Author: Jacob Maes <[email protected]> Reviewers: Prateek Maheshwari <[email protected]> Closes apache#367 from jmakes/samza-1508
apache#367) * HDFS-16791. Add getEnclosingRoot() API to filesystem interface and implementations (apache#6198) The enclosing root path is a common ancestor that should be used for temp and staging dirs as well as within encryption zones and other restricted directories. Contributed by Tom McCormick ACLOVERRIDE due to OSS backport email issue * fixing merge conflicts * Fixing backport issues * ACLOVERRIDE due to OSS backport email issue --------- Co-authored-by: Tom <[email protected]> Co-authored-by: Tom McCormick <[email protected]>
|
We're closing this stale PR because it has been open for 100 days with no activity. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
change doesBucketExistV2 to verified the acl