Skip to content

Add Kinesis Source - #31

Merged
tdas merged 16 commits into
tdas:streaming-df-kinesisfrom
zsxwing:streaming-df-kinesis
Feb 24, 2016
Merged

tdas merged 16 commits into
tdas:streaming-df-kinesisfrom
zsxwing:streaming-df-kinesis

Conversation

@zsxwing

@zsxwing zsxwing commented Feb 22, 2016

Copy link
Copy Markdown

No description provided.

throw new IllegalArgumentException("Option 'endpointUrl' is not specified")
})
val streamNames = caseInsensitiveOptions.getOrElse("streamNames", {
throw new IllegalArgumentException("Option 'streamNames' is not specified")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this is the most important option, and needs to be checked first. Also the lets make it just "streams"

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Add an example in the error message to show how names can be comma separated.

parameters: Map[String, String]): Source = {
val caseInsensitiveOptions = new CaseInsensitiveMap(parameters)
val regionName = caseInsensitiveOptions.getOrElse("regionName", {
throw new IllegalArgumentException("Option 'regionName' is not specified")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We should not check for both regionName and endpointUrl. Only one of them is sufficient. Ultimately the source should take only endpointUrl. If the user specifies region, then you can derive the endpoint from it.
Regions.getCurrentRegion.getServiceEndpoint("kinesis")

Also make the names "region" and "endpoint".

}).split(",", -1).toSet
if (streams.isEmpty || streams.exists(_.isEmpty)) {
throw new IllegalArgumentException(
"Option 'streams' is invalid. Please use comma separated string (e.g., 'stream1,stream2')")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

If the user hasnt comma separated multiple names (e.g. stream1|stream2), then this condition wont be true anyways. So this condition is about avoiding empty names, and accordingly the error statement should make it clear...

Option 'stream' is invalid, as stream names cannot be empty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants