SPARK-2745 [STREAMING] Add Java friendly methods to Duration class

[srowen]

@tdas is this what you had in mind for this JIRA? I saw this one and thought it would be easy to take care of, and helpful as I use streaming from Java.

I could do the same for Time? Happy to do so.

[SparkQA]

QA tests have started for PR 2403 at commit 4dee32e.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 2403 at commit 4dee32e.

• This patch fails unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[mateiz]

@srowen I think that in addition to these, it would be nice to have static constructor methods, such as Duration.seconds(500) and Duration.minutes(10). Mind adding those too?

[srowen]

@mateiz Will do. There's one catch. Since Duration has an accessor named milliseconds, and has a private accessor called millis from the constructor, I can't create Duration.milliseconds or Duration.millis. I could just omit this particular helper as it's pretty redundant with the constructor?

I'll also add tests, and give Time a similar treatment with Java-friendly names, and tests.

(PS the scalastyle checks fail because there appear to be binary operators without surrounding space. I believe it's necessary to disable the checks in that section of declarations, since they're not actually usages of a binary operator.)

[SparkQA]

QA tests have started for PR 2403 at commit 7dde949.

• This patch merges cleanly.

[mateiz]

Ah, sure, I think we can just skip milliseconds in that case.

For the scalastyle checks, why not use spaces around the operators instead? They should work in that context. You can just do this + that for example.

[SparkQA]

QA tests have finished for PR 2403 at commit 7dde949.

• This patch fails unit tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • class NonASCIICharacterChecker extends ScalariformChecker

[srowen]

@mateiz Ah of course. I overlooked the obvious somehow. I'm looking at why MIMA binary checks fail to see if it has a point or not now.

[SparkQA]

QA tests have started for PR 2403 at commit bda301c.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 2403 at commit bda301c.

• This patch fails unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[srowen]

I'm not sure what to make of the MIMA errors:

• the type hierarchy of object org.apache.spark.streaming.Duration has changed in new version. Missing types {scala.runtime.AbstractFunction1}
• method apply(java.lang.Object)java.lang.Object in object org.apache.spark.streaming.Duration's type has changed; was (java.lang.Object)java.lang.Object, is now: (Long)org.apache.spark.streaming.Duration
• method toString()java.lang.String in object org.apache.spark.streaming.Duration does not have a correspondent in new version

I assume this is a side-effect of adding object Duration, but I don't see why this would have changed the apply method this way. toString still definitely exists. So I wonder if this is a false positive and should be suppressed?

[mateiz]

Yeah, these seem like false positives. @ScrapCodes can you take a look and suggest how to update the MIMA rules if these are indeed false positives?

[mateiz]

Actually it may also be the case that the object doesn't work quite the way the default companion object for a case class should. Can you double check that stuff like Duration(5L) still works, as does (1L to 10L).map(Duration)? If not, we may have to put these in a helper object called Durations instead, which would still be fine (and then we could have our milliseconds back!).

[mateiz]

Yeah I think this is an actual problem, see https://issues.scala-lang.org/browse/SI-3664. Maybe we should just go with Durations for simplicity.

[ScrapCodes]

I guess we will have to exclude those in MimaExcludes.scala. If this happens too often, then may be we can ignore toString for all classes possible.

[SparkQA]

QA tests have started for PR 2403 at commit 5a9e706.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 2403 at commit 5a9e706.

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

[mateiz]

Thanks, this looks good.