Skip to content

Conversation

@SaintBacchus
Copy link
Contributor

If using spark-sql in yarn-cluster mode, print an error infomation just as the spark shell in yarn-cluster mode.

Copy link
Contributor

Choose a reason for hiding this comment

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

“shells" should be in singular form.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add comment int he SparkSQLCLIDriver class that explains there is a dependency here on the exact name? Otherwise that class could be renamed and this would break inadvertently.

@pwendell
Copy link
Contributor

I had a small comment to make this less prone to being broken. Looks good other than that comment.

@andrewor14
Copy link
Contributor

add to whitelist. This LGTM

Copy link
Contributor

Choose a reason for hiding this comment

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

I think elsewhere we use SQL (all caps). Maybe we should say "not applicable to the Spark SQL shell" here

@SparkQA
Copy link

SparkQA commented Nov 30, 2014

Test build #23953 has finished for PR 3479 at commit 8e112c5.

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

bin/spark-sql Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

A minor improvement:

# NOTE: This exact class name is matched downstream by SparkSubmit. Any changes need to be reflected there.

@pwendell
Copy link
Contributor

Just proposed a minor change to the comment. If you feel the current one is better you can leave it there. Either way LGTM.

@SparkQA
Copy link

SparkQA commented Nov 30, 2014

Test build #23958 has finished for PR 3479 at commit e6c1eb7.

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

@SparkQA
Copy link

SparkQA commented Nov 30, 2014

Test build #23959 has finished for PR 3479 at commit 35829a9.

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

@pwendell
Copy link
Contributor

pwendell commented Dec 1, 2014

LGTM - thanks I'll pull this in

@asfgit asfgit closed this in aea7a99 Dec 1, 2014
@pwendell
Copy link
Contributor

pwendell commented Dec 1, 2014

Thanks I merged this. I couldn't find you on the ASF jira to give you credit as the author of this. Can you comment here so that I can make you the assignee?

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

@SaintBacchus
Copy link
Contributor Author

Thanks for mergeing. The jira was also reported by me, and I had rename it as github.

@SaintBacchus SaintBacchus deleted the sparkSqlShell branch December 26, 2015 06:42
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.

5 participants