Skip to content

Conversation

@Jimmy-Newtron
Copy link

What changes were proposed in this pull request?

Added methods to retrieve keys or values of a pair DStream

How was this patch tested?

NONE

Added two methods that are available into the PairRDDFunctions.scala

Added two methods that are available into the PairRDDFunctions.scala
@Jimmy-Newtron Jimmy-Newtron changed the title Update PairDStreamFunctions.scala [Streaming] Update PairDStreamFunctions.scala Dec 8, 2016
@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@srowen
Copy link
Member

srowen commented Dec 8, 2016

I don't think this worth new API methods since it's so easy to do this directly. Please see http://spark.apache.org/contributing.html too. I'd close this.

@Jimmy-Newtron
Copy link
Author

I think that this new API methods is going to bring consistency between PairRDDFunctions and PairDStreamFunctions by providing the same API to access the same data.

I see the use case of a streaming application that is consuming a Kafka topic to retrieve the values as the simplest example of usage of this new API.

I accept the PR rejection, even if the new API is matching only positive aspects of the Code Review Criteria.

Positives:
Adds functionality needed by a large number of users
Simple, targeted
Negatives, Risks: NONE

Best regards

@rxin
Copy link
Contributor

rxin commented Dec 9, 2016

This is actually parity with PairRDDFunctions so I think it's OK to have them.

But again, having to add stuff like this is why we are doing structured streaming -- it's one single API for both batch and streaming.

@srowen
Copy link
Member

srowen commented Dec 9, 2016

I get it, it still seems not so worth it. The DStream API has always been mostly redundant with foreachRDD + the RDD API, and is itself maybe not something being actively updated anyway.

srowen added a commit to srowen/spark that referenced this pull request Jan 1, 2017
@srowen srowen mentioned this pull request Jan 1, 2017
@asfgit asfgit closed this in ba48812 Jan 2, 2017
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.

4 participants