Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

APEXMALHAR-2019 S3-Input Implemented S3 Input Module #263

Merged
merged 1 commit into from Jul 14, 2016

Conversation

chaithu14
Copy link
Contributor

No description provided.

}

@VisibleForTesting
protected String extractBucket(String s3uri)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

javadoc.

@chaithu14 chaithu14 force-pushed the APEXMALHAR-2019-S3-Input branch 3 times, most recently from 70f2a00 to ecbdf2a Compare May 3, 2016 06:08
@chaithu14
Copy link
Contributor Author

Addressed all the review comments.

@gauravgopi123
Copy link
Contributor

gauravgopi123 commented May 12, 2016

I see that this is using BlockReader, which is being deleted in PR #267. Please make sure that this is in line with other PR. So please hold off this till that time

@chaithu14
Copy link
Contributor Author

Re-based with the master and updated the branch.

@chaithu14
Copy link
Contributor Author

@ashwinchandrap Please review and merge.

@chaithu14 chaithu14 force-pushed the APEXMALHAR-2019-S3-Input branch 2 times, most recently from 3a24cc2 to 88fa04c Compare May 19, 2016 14:23
@gauravgopi123
Copy link
Contributor

Can we add test cases for this?

@chaithu14 chaithu14 force-pushed the APEXMALHAR-2019-S3-Input branch 2 times, most recently from 94e7959 to 13b041d Compare June 1, 2016 11:56
@chaithu14
Copy link
Contributor Author

@gauravgopi123 Thanks for reviewing. I added unit test case for S3 Input Module. But, I marked the test case as excluded. Because to run this test case, the prerequisites are amazon credentials and hadoop.

@yogidevendra
Copy link
Contributor

@chaithu14 Please make changes to the readEntity() javadoc and please rebase this branch.
If there are no comments from other community members then I will go ahead and merge this.

@yogidevendra
Copy link
Contributor

@chaithu14 Could you please mark the new classes with @ evolving?

@chaithu14
Copy link
Contributor Author

@yogidevendra : Incorporated review comments and marked the new classes with @ evolving.

@yogidevendra
Copy link
Contributor

Looks fine to me. I will merge this tomorrow.

@asfgit asfgit merged commit 5de26e4 into apache:master Jul 14, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants