You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by kaklakariada <gi...@git.apache.org> on 2016/02/16 10:50:20 UTC
[GitHub] spark pull request: [SPARK-10969] [Streaming] [Kinesis] Allow spec...
GitHub user kaklakariada opened a pull request:
https://github.com/apache/spark/pull/11215
[SPARK-10969] [Streaming] [Kinesis] Allow specifying separate credentials for Kinesis and DynamoDB
`KinesisUtils.createStream()` allows specifying only one set of AWS credentials that will be used by Amazon Kinesis Client Library (KCL) for accessing Kinesis, DynamoDB and CloudWatch.
#### Motivation
In a scenario where one needs to read from a Kinesis Stream owned by a different AWS account the user usually has minimal rights (i.e. only read from the stream). In this case creating the DynamoDB table in KCL will fail. Instead the DynamoDB table should be created in your own AWS account.
#### Proposal
This PR adds a new serializable class `AWSCredentialPool` containing credentials for Kinesis, DynamoDB and CloudWatch that is passed as a parameter to `KinesisUtils.createStream()`. These credentials are then passed to the constructor of `KinesisClientLibConfiguration`.
@tdas @cfregly please review if this PR is OK.
This contribution is @ajayrammohan and my original work and we license the work to the project under the project's open source license.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ajayrammohan/spark SPARK-10969
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/11215.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #11215
----
commit 517dfb4f00405b967c244b2d75756fc33f02021f
Author: Ajay Rammohan <aj...@elektrobit.com>
Date: 2016-02-10T13:06:30Z
SPARK-10969- Added new credentials Pool class to handle multiple AWS credentials
commit d50ce058f24e938eb2d4c733dffc517bbb410efa
Author: Ajay Rammohan <aj...@elektrobit.com>
Date: 2016-02-10T13:06:58Z
SPARK-10969- Changed interface to handle the credential Pool as parameter
commit 027452253e472180b2cd6d01bc641b6e15e33d5e
Author: kaklakariada <ka...@users.noreply.github.com>
Date: 2016-02-10T14:01:54Z
SPARK-10969 fix scala compilation error
commit f3093ebcaed4919803f6cbb1dbfb6c58d1310ec0
Author: kaklakariada <ka...@users.noreply.github.com>
Date: 2016-02-10T15:26:31Z
SPARK-10969 pass AWSCredentialsPool to KinesisReceiver and pass AWS credentials to KinesisClientLibConfiguration
commit 0e9db8c398d8f6a60a98d226139704a835c16326
Author: Ajay Rammohan <aj...@elektrobit.com>
Date: 2016-02-11T07:53:52Z
SPARK-10969- adapted javadoc
commit 44cdc8e6d349402755f7e5bbf41a8ffb77702218
Author: Ajay Rammohan <aj...@elektrobit.com>
Date: 2016-02-12T07:46:31Z
SPARK-10969- added new test case to test new api
commit 552831412ff10268aeccd6ede37b08c4d5ebfa07
Author: Ajay Rammohan <aj...@elektrobit.com>
Date: 2016-02-12T07:47:45Z
SPARK-10969- added new methods to get profile based credentials and cleanup tables created in separate accounts
commit 892f9c99a86fd00bdbd17e2de47370f765db0ab9
Author: Ajay Rammohan <aj...@elektrobit.com>
Date: 2016-02-15T07:29:13Z
SPARK-10969 - Adapted DynamoDB cleanup. Now checks for tables in multiple accounts.
commit d7c29f9cd4654986d720236155960ec7497aa1ef
Author: kaklakariada <ka...@users.noreply.github.com>
Date: 2016-02-15T14:25:34Z
SPARK-10969 - Allow passing a credential pool from Java, add Java unit tests
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-10969] [Streaming] [Kinesis] Allow spec...
Posted by soulcutter <gi...@git.apache.org>.
Github user soulcutter commented on the pull request:
https://github.com/apache/spark/pull/11215#issuecomment-190273914
I like the idea of being able to provide separate credentials for services, however it seems to me like a more-common use case is still going to be providing a single set of credentials for all services. It would be nice if the way to do so was still as straightforward as it is currently.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-10969] [Streaming] [Kinesis] Allow spec...
Posted by soulcutter <gi...@git.apache.org>.
Github user soulcutter commented on the pull request:
https://github.com/apache/spark/pull/11215#issuecomment-192840074
Welp I think you're right :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-10969] [Streaming] [Kinesis] Allow spec...
Posted by kaklakariada <gi...@git.apache.org>.
Github user kaklakariada commented on the pull request:
https://github.com/apache/spark/pull/11215#issuecomment-192823405
@soulcutter Thank you very much for your review and sorry for the delay!
Please correct me if I am wrong, but in test [testKinesisStreamWithSimpleAwsCredentials](https://github.com/ajayrammohan/spark/blob/SPARK-10969/extras/kinesis-asl/src/test/java/org/apache/spark/streaming/kinesis/JavaKinesisStreamSuite.java#L50) we test, that you still can use a single set of credentials with the old API.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #11215: [SPARK-10969] [Streaming] [Kinesis] Allow specifying sep...
Posted by kaklakariada <gi...@git.apache.org>.
Github user kaklakariada commented on the issue:
https://github.com/apache/spark/pull/11215
Solved with https://issues.apache.org/jira/browse/SPARK-19911 / #17250, see [this comment](https://issues.apache.org/jira/browse/SPARK-10969?focusedCommentId=16266374&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16266374).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #11215: [SPARK-10969] [Streaming] [Kinesis] Allow specify...
Posted by kaklakariada <gi...@git.apache.org>.
Github user kaklakariada closed the pull request at:
https://github.com/apache/spark/pull/11215
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-10969] [Streaming] [Kinesis] Allow spec...
Posted by soulcutter <gi...@git.apache.org>.
Github user soulcutter commented on a diff in the pull request:
https://github.com/apache/spark/pull/11215#discussion_r54432870
--- Diff: extras/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisTestUtils.scala ---
@@ -221,13 +237,51 @@ private[kinesis] object KinesisTestUtils {
case Failure(e) =>
throw new Exception(
s"""
- |Kinesis tests enabled using environment variable $envVarNameForEnablingTests
- |but could not find AWS credentials. Please follow instructions in AWS documentation
- |to set the credentials in your system such that the DefaultAWSCredentialsProviderChain
- |can find the credentials.
+ |Kinesis tests that actually send data has been enabled by setting the environment
+ |variable $envVarNameForEnablingTests to 1. This will create Kinesis Streams and
+ |DynamoDB tables in AWS. Please be aware that this may incur some AWS costs.
+ |By default, the tests use the endpoint URL $defaultEndpointUrl to create Kinesis streams.
+ |To change this endpoint URL to a different region, you can set the environment variable
+ |$endVarNameForEndpoint to the desired endpoint URL
+ |(e.g. $endVarNameForEndpoint="https://kinesis.us-west-2.amazonaws.com").
""".stripMargin)
}
}
+
+ def getAWSProfileCredentials(profile: String): AWSCredentials = {
+ Try { new ProfileCredentialsProvider(profile).getCredentials() } match {
+ case Success(cred) => {
+ if (profile.equals(KinesisTestUtils.DYNAMODB_PROFILE)) {
+ KinesisTestUtils.cleanUpProfileDynamoTables = true;
+ }
+ cred
+ }
+ case Failure(e) =>
+ println(
+ s"""
+ |Could not load the AWS credentials for profile $profile.
+ |
+ |Credentialpool tests verify the ability to uses Kinesis stream, Dynamo DB and CloudWatch Metrics in
+ |separate AWS accounts. This requires a separate set of AWS credentials to be stored in system with the profile
+ |names "dynamoDB" and "cloudWatch" along with the default credentials. The test assumes that the Kinesis stream
+ |in the In absence of this only the underlying API will be tested.
--- End diff --
This sentence looks like it got mangled during editing somehow.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-10969] [Streaming] [Kinesis] Allow spec...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11215#issuecomment-184601498
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org