[Streaming][Kafka][SPARK-8389] Example of getting offset ranges out o…

[koeninger]

…f the existing java direct stream api

[SparkQA]

Test build #34997 has finished for PR 6846 at commit 3f3c57a.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[srowen]

LGTM

[srowen]

Nit, not worth fixing unless you make another change: space after the cast.
Was the intent of the JIRA to add something to an example? this is an addition a test which is even better, but I also read that it's users who want to know about this too.

[koeninger]

Yeah, there's a separate JIRA for updating the doc. Will probably do that
on the plane back from spark conf, at least for the java/scala side
On Jun 17, 2015 12:30 AM, "Sean Owen" notifications@github.com wrote:

In
external/kafka/src/test/java/org/apache/spark/streaming/kafka/JavaDirectKafkaStreamSuite.java
#6846 (comment):

@@ -89,6 +90,16 @@ public void testKafkaStream() throws InterruptedException {
StringDecoder.class,
kafkaParams,
topicToSet(topic1)

• ).transformToPair(

•    // Make sure you can get offset ranges from the rdd
  

•    new Function<JavaPairRDD<String, String>, JavaPairRDD<String, String>>() {
  

•      @Override
  

•      public JavaPairRDD<String, String> call(JavaPairRDD<String, String> rdd) throws Exception {
  

•        OffsetRange[] offsets = ((HasOffsetRanges)rdd.rdd()).offsetRanges();
  

Nit, not worth fixing unless you make another change: space after the cast.
Was the intent of the JIRA to add something to an example? this is an
addition a test which is even better, but I also read that it's users who
want to know about this too.

—
Reply to this email directly or view it on GitHub
https://github.com/apache/spark/pull/6846/files#r32601142.

[srowen]

LGTM; do you consider this to resolve SPARK-8389 or is there more coming?

[koeninger]

Thanks. Not sure if the python side of things is going to continue on SPARK-8389 or on SPARK-8337, but I think this takes care of the java side for now.

[tdas]

Was this committed to branch-1.4?

[koeninger]

My original pr was against master

On Fri, Jun 19, 2015 at 7:27 PM, Tathagata Das notifications@github.com
wrote:

Was this committed to branch-1.4?

—
Reply to this email directly or view it on GitHub
#6846 (comment).

[tdas]

@srowen This was not committed to branch-1.4 even though the target version of Spark 1.4.1 in the JIRA. Causing merge conflicts for other PR #6863 in branch-1.4

Also the JIRA should have been 8390 as this PR is not about exposing offsets in Java API (8389, offsets were already exposed, my JIRA title in 8389 was incorrect, I will fix that), but more about documenting (8390).

[tdas]

@koeninger naah, not your fault, we backport stuff at the time of merging.
@srowen dont worry about it, i am cherry-picking this PR and #6863 to branch 1.4

[srowen]

Ok it looked like something that did not require a back port to me but yes please go ahead. It is really fixes and docs that go back right?

[tdas]

Yeah, it wasnt obvious from the patch that it should be backported. Well, if it is not related to a new feature, then I generally back port it, especially when it is so close to the 1.4 release where we might still be merging stuff to branch-1.4 heavily.

Isnt it a good idea to generally follow a policy that if JIRA target version is set to have versions in past branches, whoever is merging should try to merge it to the previous branch as well? If we dont have a policy, then shouldnt we have something as a way to better way to communicate intent between committers?

[srowen]

Yes, assuming Target Version is meaningful, and it was here since you set it. I just overlooked it.

[tdas]

That is a valid question. We all arent super rigorous about updating
Targetting version for all JIRAs timely. On one hand we should try to do
that. On another hand, if it seems to be set properly at the current time,
we should try to do it.

May be these should be in the guidelines for committers :)

On Mon, Jun 22, 2015 at 1:29 AM, Sean Owen notifications@github.com wrote:

Yes, assuming Target Version is meaningful, and it was here since you set
it. I just overlooked it.

—
Reply to this email directly or view it on GitHub
#6846 (comment).