You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by liu-zhaokun <gi...@git.apache.org> on 2017/08/08 06:41:52 UTC

[GitHub] spark pull request #18879: [SPARK-21662] modify the appname to [SparkSQL::lo...

GitHub user liu-zhaokun opened a pull request:

    https://github.com/apache/spark/pull/18879

    [SPARK-21662] modify the appname to [SparkSQL::localHostName] instead of [SparkSQL::lP]	

    [https://issues.apache.org/jira/browse/SPARK-21662](https://issues.apache.org/jira/browse/SPARK-21662)
    As it says "If user doesn't specify the appName, we want to get [SparkSQL::localHostName]" in SparkSqlEnv.scala,line 38,appname should be SparkSQL::localHostName,but it is SparkSQL::lP in fact.So I modify the localHostName method to get localhost with getCanonicalHostName method.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/liu-zhaokun/spark master0808

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/18879.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #18879
    
----
commit ab2248587d1a761058411c9a7f39e2da13f433bd
Author: liuzhaokun <li...@zte.com.cn>
Date:   2017-08-08T06:37:35Z

    [SPARK-21662] modify the appname to [SparkSQL::localHostName] instead of [SparkSQL::lP]

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18879: [SPARK-21662] modify the appname to [SparkSQL::localHost...

Posted by liu-zhaokun <gi...@git.apache.org>.
Github user liu-zhaokun commented on the issue:

    https://github.com/apache/spark/pull/18879
  
    @jiangxb1987 
    BTW,how to find the release branch of Hive-0.13.1a which spark1.4.1 compiled with?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18879: [SPARK-21662] modify the appname to [SparkSQL::localHost...

Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on the issue:

    https://github.com/apache/spark/pull/18879
  
    I think @srowen means you should change the method name, not only the comment.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18879: [SPARK-21662] modify the appname to [SparkSQL::localHost...

Posted by liu-zhaokun <gi...@git.apache.org>.
Github user liu-zhaokun commented on the issue:

    https://github.com/apache/spark/pull/18879
  
    @srowen 
    I have modified the annotation,would you like to review it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18879: [SPARK-21662] modify the appname to [SparkSQL::localHost...

Posted by liu-zhaokun <gi...@git.apache.org>.
Github user liu-zhaokun commented on the issue:

    https://github.com/apache/spark/pull/18879
  
    Thanks for your patience.Would you  @srowen  agree with @jiangxb1987 .


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18879: [SPARK-21662] modify the appname to [SparkSQL::lo...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18879#discussion_r132123263
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLEnv.scala ---
    @@ -35,7 +35,7 @@ private[hive] object SparkSQLEnv extends Logging {
       def init() {
         if (sqlContext == null) {
           val sparkConf = new SparkConf(loadDefaults = true)
    -      // If user doesn't specify the appName, we want to get [SparkSQL::localHostName] instead of
    +      // If user doesn't specify the appName, we will get [SparkSQL::IP] instead of
    --- End diff --
    
    What is SparkSQL::IP?
    I think this is not worth a JIRA / PR relative to the time spent looking at it
    What problem does this cause a user?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18879: [SPARK-21662] modify the appname to [SparkSQL::localHost...

Posted by wangyum <gi...@git.apache.org>.
Github user wangyum commented on the issue:

    https://github.com/apache/spark/pull/18879
  
    retest this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #18879: [SPARK-21662] modify the appname to [SparkSQL::localHost...

Posted by liu-zhaokun <gi...@git.apache.org>.
Github user liu-zhaokun commented on the issue:

    https://github.com/apache/spark/pull/18879
  
    @srowen  @jiangxb1987 
    If we can't change this logic,I think we should change the annotation,because it says "If user doesn't specify the appName, we want to get [SparkSQL::localHostName]" ,it isn't in line with the facts.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #18879: [SPARK-21662] modify the appname to [SparkSQL::localHost...

Posted by liu-zhaokun <gi...@git.apache.org>.
Github user liu-zhaokun commented on the issue:

    https://github.com/apache/spark/pull/18879
  
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18879: [SPARK-21662] modify the appname to [SparkSQL::localHost...

Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on the issue:

    https://github.com/apache/spark/pull/18879
  
    I'm not convinced this deserves a separated PR because it's neither bugfix nor any improvement. Further more, the method `Utils.localHostName()` is used in many other places, we can't change this unless we have investigated on each place it appears.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18879: [SPARK-21662] modify the appname to [SparkSQL::localHost...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/18879
  
    The method name is misleading internally, but we can't change this logic.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18879: [SPARK-21662] modify the appname to [SparkSQL::lo...

Posted by liu-zhaokun <gi...@git.apache.org>.
Github user liu-zhaokun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18879#discussion_r132126001
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLEnv.scala ---
    @@ -35,7 +35,7 @@ private[hive] object SparkSQLEnv extends Logging {
       def init() {
         if (sqlContext == null) {
           val sparkConf = new SparkConf(loadDefaults = true)
    -      // If user doesn't specify the appName, we want to get [SparkSQL::localHostName] instead of
    +      // If user doesn't specify the appName, we will get [SparkSQL::IP] instead of
    --- End diff --
    
    @srowen 
    It is not accurate,because the comment is different from the appname in web UI.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18879: [SPARK-21662] modify the appname to [SparkSQL::lo...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/18879


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18879: [SPARK-21662] modify the appname to [SparkSQL::localHost...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18879
  
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org