changed scala example from java "style" to scala by mantovani · Pull Request #20458 · apache/spark

mantovani · 2018-01-31T19:49:19Z

What changes were proposed in this pull request?

I changed an example wrote in Scala using Java "style" to Scala "style".

How was this patch tested?

It's not necessary test, the language documentation should be enough. For the documentation propose.

http://www.scala-lang.org/api/2.9.2/scala/App.html
Please review http://spark.apache.org/contributing.html before opening a pull request.

mgaido91 · 2018-01-31T20:30:15Z

the example is written in Java, not in Scala. I think you can close this PR, thanks.

srowen · 2018-01-31T20:33:57Z

- String logFile = "YOUR_SPARK_HOME/README.md"; // Should be some file on your system
- SparkSession spark = SparkSession.builder().appName("Simple Application").getOrCreate();
- Dataset<String> logData = spark.read().textFile(logFile).cache();
+public class SimpleApp extends App {


Yes, this isn't valid Java, and it's supposed to be Java. It was correct before.

Sorry, I updated the wrong section.

updated the right section, Scala.

mantovani · 2018-01-31T20:46:52Z

I just changed to the right section.

srowen · 2018-01-31T20:54:42Z

IIRC extending App won't work, or didn't. The example is already correct.

mantovani · 2018-01-31T21:02:00Z

It's not just "will" work, it's working. I never said that it wasn't working, the Scala example was wrote using Java "style" not Scala "style" which . Which you don't create a main method but extends App.

vanzin · 2018-01-31T21:19:37Z

It's not just "will" work, it's working.

https://issues.apache.org/jira/browse/SPARK-4170

mantovani · 2018-01-31T22:11:23Z

Hello Marcel, This is an old bug and was fixed long time ago, I just runned the following example and got the correct result: DemoBug: rslt1 = 3 rslt2 = 3 Doesn't make sense use Java traditional way "main(...)" just because 4 years ago the "Scala approach" had a bug. Sometimes we can't use feature because of bugs, it's fine but we should move on when they are fixed.

…

On Wed, Jan 31, 2018 at 4:20 PM, Marcelo Vanzin ***@***.***> wrote: It's not just "will" work, it's working. https://issues.apache.org/jira/browse/SPARK-4170 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#20458 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAC5fS39XmaJETebqFaYm7SzKOtDk1KVks5tQNkWgaJpZM4R0i6_> .

--

-- Daniel de Oliveira Mantovani Perl Evangelist/Data Hacker +1 786 459 1341

vanzin · 2018-01-31T22:15:18Z

Did you try it with Scala 2.10? That's still supported by Spark.

mantovani · 2018-01-31T22:22:25Z

I just tested with Scala 2.10.6 and Spark 2.1.1 and worked perfect. Since this patch won't be applied for Spark 1.x documentation, it would be fine. 18/01/31 17:21:28 INFO DAGScheduler: Job 1 finished: count at Bootstrap.scala:14, took 0.024446 s DemoBug: rslt1 = 3 rslt2 = 3

…

On Wed, Jan 31, 2018 at 5:16 PM, Marcelo Vanzin ***@***.***> wrote: Did you try it with Scala 2.10? That's still supported by Spark. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#20458 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAC5fSAOjXpf1HNiOCQhx_BUL4GxaC4gks5tQOYggaJpZM4R0i6_> .

--

-- Daniel de Oliveira Mantovani Perl Evangelist/Data Hacker +1 786 459 1341

srowen · 2018-01-31T23:40:38Z

Yeah, that's what I was thinking of. Subclasses of App don't necessarily work in Spark (not just Scala). I don't know if that has changed as Spark and Scala moved forward. While that's an interesting item to investigate, here the question is changing one example to use App, while none of the others do and there's a reason not to. We don't want to do that.

mantovani · 2018-02-01T00:05:16Z

If I change all examples of the documentation will be accept ?

…

On Wed, Jan 31, 2018 at 6:41 PM, Sean Owen ***@***.***> wrote: Yeah, that's what I was thinking of. Subclasses of App don't necessarily work in Spark (not just Scala). I don't know if that has changed as Spark and Scala moved forward. While that's an interesting item to investigate, here the question is changing one example to use App, while none of the others do and there's a reason not to. We don't want to do that. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#20458 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAC5fXruoTYCd0hMD4wsXtMSNUYB-pz5ks5tQPohgaJpZM4R0i6_> .

--

-- Daniel de Oliveira Mantovani Perl Evangelist/Data Hacker +1 786 459 1341

srowen · 2018-02-01T00:18:18Z

The point is that it won't always work in Spark, so, certainly not unless you test them. I just don't think it's worth it, especially as it's setting an expectation as an example that it is the best way to do it.

mantovani · 2018-02-01T00:21:05Z

Sean, I already tested. Can you please be more specific how should be the tests ? I just tested on Spark 2.1.1 and Scala 2.11 and 2.10.

On Wed, Jan 31, 2018 at 19:19 Sean Owen ***@***.***> wrote: The point is that it won't always work in Spark, so, certainly not unless you test them. I just don't think it's worth it, especially as it's setting an expectation as an example that it is the best way to do it. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#20458 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAC5fQJd6YYF8AI0tijFEkzDKExgiABUks5tQQL_gaJpZM4R0i6_> .

--

…

-- Daniel de Oliveira Mantovani Perl Evangelist/Data Hacker +1 786 459 1341

AmplabJenkins · 2018-03-03T02:22:36Z

Can one of the admins verify this patch?

Closesapache#20458Closesapache#20530Closesapache#20557Closesapache#20966Closesapache#20857Closesapache#19694Closesapache#18227Closesapache#20683Closesapache#20881Closesapache#20347Closesapache#20825Closesapache#20078Closesapache#21281Closesapache#19951Closesapache#20905Closesapache#20635 Author: Sean Owen <srowen@gmail.com> Closesapache#21303 from srowen/ClosePRs.

changed scala example from java "style" to scala
bc708cb

srowen reviewed Jan 31, 2018
View reviewed changes

Update quick-start.md
623c473
updated the right section, Scala.

srowen mentioned this pull request May 11, 2018
[BUILD] Close stale PRs #21303
Closed

asfgit closed this in 348ddfdMay 12, 2018

Uh oh!

Conversation

mantovani commented Jan 31, 2018

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

mgaido91 commented Jan 31, 2018

Uh oh!

srowenJan 31, 2018

Choose a reason for hiding this comment

Uh oh!

mantovaniJan 31, 2018

Choose a reason for hiding this comment

Uh oh!

mantovani commented Jan 31, 2018

Uh oh!

srowen commented Jan 31, 2018

Uh oh!

mantovani commented Jan 31, 2018

Uh oh!

vanzin commented Jan 31, 2018

Uh oh!

mantovani commented Jan 31, 2018 via email

Uh oh!

vanzin commented Jan 31, 2018

Uh oh!

mantovani commented Jan 31, 2018 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srowen commented Jan 31, 2018

Uh oh!

mantovani commented Feb 1, 2018 via email

Uh oh!

srowen commented Feb 1, 2018

Uh oh!

mantovani commented Feb 1, 2018 via email

Uh oh!

AmplabJenkins commented Mar 3, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mantovani commented Jan 31, 2018 via email •
edited
Loading