Skip to content

Conversation

@dongjoon-hyun
Copy link
Member

What changes were proposed in this pull request?

Spark Shell provides an easy way to use Spark in Scala environment. This PR adds reset command to a blocked list, also cleaned up according to the Scala coding style.

scala> sc
res0: org.apache.spark.SparkContext = org.apache.spark.SparkContext@718fad24
scala> :reset
scala> sc
<console>:11: error: not found: value sc
       sc
       ^

If we blocks reset, Spark Shell works like the followings.

scala> :reset
reset: no such command.  Type :help for help.
scala> :re
re is ambiguous: did you mean :replay or :require?

How was this patch tested?

Manual. Run bin/spark-shell and type :reset.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-14102] Block reset command in SparkShell [SPARK-14102][CORE] Block reset command in SparkShell Mar 23, 2016
@dongjoon-hyun
Copy link
Member Author

In Jira, I chose Spark Shell as a component, but here CORE is the nearest one.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see, hm, this is not actually tricky to implement now. Well, it's not a crazy idea, anyone have opinions on blocking :reset? People really just shouldn't call it, and should restart the shell.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for review, @srowen .

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I didn't search properly to find the duplicated JIRA issue: SPARK-6335.
Thank you for correcting me there.

@SparkQA
Copy link

SparkQA commented Mar 24, 2016

Test build #53965 has finished for PR 11920 at commit 8b14dba.

  • This patch fails from timeout after a configured wait of 250m.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 24, 2016

Test build #53995 has finished for PR 11920 at commit 7a438ae.

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

@SparkQA
Copy link

SparkQA commented Mar 24, 2016

Test build #54053 has finished for PR 11920 at commit d071621.

  • This patch fails from timeout after a configured wait of 250m.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 25, 2016

Test build #54123 has finished for PR 11920 at commit 62a4020.

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

@dongjoon-hyun
Copy link
Member Author

Hi, @srowen .
Recently Jenkins was a little bit flakey.
How do you think about this PR now?
The code is the same. It is just rebased to pass the test.

@srowen
Copy link
Member

srowen commented Mar 25, 2016

OK. If it's pretty simple and there is no good reason to call reset (since it kills the SparkContext) maybe it is worth blocking after all.

@dongjoon-hyun
Copy link
Member Author

Thank you for the positive feedback. Indeed, it was just one-line change to block reset.
All the other changes in that file are just about ScalaStyle corrections.
Could you merge this PR, then?

@srowen
Copy link
Member

srowen commented Mar 28, 2016

Merged to master

@asfgit asfgit closed this in b66aa90 Mar 28, 2016
@dongjoon-hyun
Copy link
Member Author

Thank you, @srowen !

@dongjoon-hyun dongjoon-hyun deleted the SPARK-14102 branch May 12, 2016 00:54
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.

3 participants