You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by judynash <gi...@git.apache.org> on 2014/12/11 07:39:53 UTC

[GitHub] spark pull request: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

GitHub user judynash opened a pull request:

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

    [SparkSQL, Thrift] SPARK-4700: Add HTTP protocol spark thrift server

    Add HTTP protocol support and test cases to spark thrift server, so users can deploy thrift server in both TCP and http mode. 

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

    $ git pull https://github.com/judynash/spark master

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

    https://github.com/apache/spark/pull/3672.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 #3672
    
----
commit 377532cdff819010aef1786f84c987eddb63af45
Author: judynash <ju...@microsoft.com>
Date:   2014-12-11T06:32:13Z

    add HTTP protocol spark thrift server

----


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66725464
  
      [Test build #24389 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24389/consoleFull) for   PR 3672 at commit [`2e9c11c`](https://github.com/apache/spark/commit/2e9c11c73585e7484ba07ab0b35c24a084a54021).
     * This patch merges cleanly.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-67120862
  
      [Test build #24483 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24483/consoleFull) for   PR 3672 at commit [`526315d`](https://github.com/apache/spark/commit/526315d0cd3dbf34f7e6b5a3ea6d5187bf96cccb).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class Analyzer(catalog: Catalog, registry: FunctionRegistry, caseSensitive: Boolean)`



---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66916125
  
    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 pull request: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by judynash <gi...@git.apache.org>.
Github user judynash commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66726073
  
    I have addressed the feedbacks (thank you Cheng!) and added documentation. Let me know if there is anything else. Thanks again!


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66587060
  
    This LGTM except for several minor styling issue. Thanks for working on this!


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66733569
  
    Did a double check and spotted a few more minor styling issues. This PR should be ready to go once those issue are fixed. Thanks!


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#discussion_r21729067
  
    --- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
    @@ -140,7 +160,8 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
     
         def captureLogOutput(line: String): Unit = {
           buffer += line
    -      if (line.contains("ThriftBinaryCLIService listening on")) {
    +      if (line.contains("ThriftBinaryCLIService listening on") ||
    +        line.contains("Started ThriftHttpCLIService in http")) {
    --- End diff --
    
    Two more spaces for indentation.


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

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

    https://github.com/apache/spark/pull/3672#discussion_r21662942
  
    --- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
    @@ -113,7 +118,8 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
     
       def startThriftServer(
           port: Int,
    -      serverStartTimeout: FiniteDuration = 1.minute)(
    +      serverStartTimeout: FiniteDuration = 1.minute,
    +      httpMode: Boolean = false )(
    --- End diff --
    
    Please remove the space before `)`


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

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

    https://github.com/apache/spark/pull/3672#discussion_r21661967
  
    --- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
    @@ -140,7 +159,7 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
     
         def captureLogOutput(line: String): Unit = {
           buffer += line
    -      if (line.contains("ThriftBinaryCLIService listening on")) {
    +      if (line.contains("ThriftBinaryCLIService listening on") || line.contains("Started ThriftHttpCLIService in http")) {
    --- End diff --
    
    100 column exceeded, please wrap.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66944216
  
    The most recent test failures should be caused by other components. Let's wait for a moment. This PR now LGTM.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#discussion_r21729057
  
    --- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
    @@ -121,15 +128,28 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
         val warehousePath = getTempFilePath("warehouse")
         val metastorePath = getTempFilePath("metastore")
         val metastoreJdbcUri = s"jdbc:derby:;databaseName=$metastorePath;create=true"
    +
         val command =
    -      s"""$startScript
    -         |  --master local
    -         |  --hiveconf hive.root.logger=INFO,console
    -         |  --hiveconf ${ConfVars.METASTORECONNECTURLKEY}=$metastoreJdbcUri
    -         |  --hiveconf ${ConfVars.METASTOREWAREHOUSE}=$warehousePath
    -         |  --hiveconf ${ConfVars.HIVE_SERVER2_THRIFT_BIND_HOST}=${"localhost"}
    -         |  --hiveconf ${ConfVars.HIVE_SERVER2_THRIFT_PORT}=$port
    -       """.stripMargin.split("\\s+").toSeq
    +      if (httpMode) {
    +          s"""$startScript
    +           |  --master local
    +           |  --hiveconf hive.root.logger=INFO,console
    +           |  --hiveconf ${ConfVars.METASTORECONNECTURLKEY}=$metastoreJdbcUri
    +           |  --hiveconf ${ConfVars.METASTOREWAREHOUSE}=$warehousePath
    +           |  --hiveconf ${ConfVars.HIVE_SERVER2_THRIFT_BIND_HOST}=localhost
    +           |  --hiveconf ${ConfVars.HIVE_SERVER2_TRANSPORT_MODE}=http
    +           |  --hiveconf ${ConfVars.HIVE_SERVER2_THRIFT_HTTP_PORT}=$port
    --- End diff --
    
    Please indent these lines according to the format in the `else` branch.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by judynash <gi...@git.apache.org>.
Github user judynash commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-67274367
  
    Michael – thanks for checking in! Will this be part of 1.2 as well?
    
    Wondering since I didn’t see the merge on branch 1.2.
    
    From: Michael Armbrust [mailto:notifications@github.com]
    Sent: Tuesday, December 16, 2014 12:37 PM
    To: apache/spark
    Cc: Judy Nash
    Subject: Re: [spark] [SQL] SPARK-4700: Add HTTP protocol spark thrift server (#3672)
    
    
    LGTM, thanks for working on this! Merging to master and 1.2.
    
    —
    Reply to this email directly or view it on GitHub<https://github.com/apache/spark/pull/3672#issuecomment-67227330>.



---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

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

    https://github.com/apache/spark/pull/3672#discussion_r21662905
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -85,10 +86,22 @@ private[hive] class HiveThriftServer2(hiveContext: HiveContext)
         setSuperField(this, "cliService", sparkSqlCliService)
         addService(sparkSqlCliService)
     
    -    val thriftCliService = new ThriftBinaryCLIService(sparkSqlCliService)
    -    setSuperField(this, "thriftCLIService", thriftCliService)
    -    addService(thriftCliService)
    +    if (isHTTPTransportMode(hiveConf)){
    +      val thriftCliService = new ThriftHttpCLIService(sparkSqlCliService)
    +      setSuperField(this, "thriftCLIService", thriftCliService)
    +      addService(thriftCliService)
    +    } else {
    +      val thriftCliService = new ThriftBinaryCLIService(sparkSqlCliService)
    +      setSuperField(this, "thriftCLIService", thriftCliService)
    +      addService(thriftCliService)
    +    }
     
         initCompositeService(hiveConf)
       }
    +
    +  private def isHTTPTransportMode(hiveConf: HiveConf): Boolean = {
    +    val transportMode: String = hiveConf.getVar(ConfVars.HIVE_SERVER2_TRANSPORT_MODE)
    +    return transportMode.equalsIgnoreCase("http")
    --- End diff --
    
    In Scala we don't need `return` here.


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

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

    https://github.com/apache/spark/pull/3672#issuecomment-66579378
  
    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 pull request: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

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

    https://github.com/apache/spark/pull/3672#discussion_r21720621
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -85,10 +86,22 @@ private[hive] class HiveThriftServer2(hiveContext: HiveContext)
         setSuperField(this, "cliService", sparkSqlCliService)
         addService(sparkSqlCliService)
     
    -    val thriftCliService = new ThriftBinaryCLIService(sparkSqlCliService)
    -    setSuperField(this, "thriftCLIService", thriftCliService)
    -    addService(thriftCliService)
    +    if (isHTTPTransportMode(hiveConf)){
    +      val thriftCliService = new ThriftHttpCLIService(sparkSqlCliService)
    +      setSuperField(this, "thriftCLIService", thriftCliService)
    +      addService(thriftCliService)
    +    } else {
    +      val thriftCliService = new ThriftBinaryCLIService(sparkSqlCliService)
    +      setSuperField(this, "thriftCLIService", thriftCliService)
    +      addService(thriftCliService)
    +    }
     
         initCompositeService(hiveConf)
       }
    +
    +  private def isHTTPTransportMode(hiveConf: HiveConf): Boolean = {
    +    val transportMode: String = hiveConf.getVar(ConfVars.HIVE_SERVER2_TRANSPORT_MODE)
    +    return transportMode.equalsIgnoreCase("http")
    --- End diff --
    
    Yea, usually we don't use `return` unless absolutely necessary. Everything is an expression in Scala, and the last expression of the function body is the return value.
    
    BTW, here is a brief [Spark code style guide] [1].
    
    [1]: https://cwiki.apache.org/confluence/display/SPARK/Spark+Code+Style+Guide


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#discussion_r21729040
  
    --- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
    @@ -70,11 +70,17 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
         port
       }
     
    -  def withJdbcStatement(serverStartTimeout: FiniteDuration = 1.minute)(f: Statement => Unit) {
    +  def withJdbcStatement(serverStartTimeout: FiniteDuration = 1.minute, httpMode: Boolean = false)(f: Statement => Unit) {
    --- End diff --
    
    100 column exceeded, please wrap this line according to the format of `startThriftServer` function.


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

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

    https://github.com/apache/spark/pull/3672#discussion_r21663017
  
    --- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
    @@ -121,15 +127,28 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
         val warehousePath = getTempFilePath("warehouse")
         val metastorePath = getTempFilePath("metastore")
         val metastoreJdbcUri = s"jdbc:derby:;databaseName=$metastorePath;create=true"
    +
         val command =
    -      s"""$startScript
    -         |  --master local
    -         |  --hiveconf hive.root.logger=INFO,console
    -         |  --hiveconf ${ConfVars.METASTORECONNECTURLKEY}=$metastoreJdbcUri
    -         |  --hiveconf ${ConfVars.METASTOREWAREHOUSE}=$warehousePath
    -         |  --hiveconf ${ConfVars.HIVE_SERVER2_THRIFT_BIND_HOST}=${"localhost"}
    -         |  --hiveconf ${ConfVars.HIVE_SERVER2_THRIFT_PORT}=$port
    -       """.stripMargin.split("\\s+").toSeq
    +      if (httpMode){
    +          s"""$startScript
    +           |  --master local
    +           |  --hiveconf hive.root.logger=INFO,console
    +           |  --hiveconf ${ConfVars.METASTORECONNECTURLKEY}=$metastoreJdbcUri
    +           |  --hiveconf ${ConfVars.METASTOREWAREHOUSE}=$warehousePath
    +           |  --hiveconf ${ConfVars.HIVE_SERVER2_THRIFT_BIND_HOST}=${"localhost"}
    +           |  --hiveconf ${ConfVars.HIVE_SERVER2_TRANSPORT_MODE}=${"http"}
    --- End diff --
    
    The `${...}` wrapper is not needed in this line as well as the line above. It's safe to use double quotes within `"""..."""`.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66731361
  
      [Test build #24389 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24389/consoleFull) for   PR 3672 at commit [`2e9c11c`](https://github.com/apache/spark/commit/2e9c11c73585e7484ba07ab0b35c24a084a54021).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by judynash <gi...@git.apache.org>.
Github user judynash commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66843978
  
    Fixed the styling issue and generated SQL Programming Guide doc. See screenshot http://i.imgur.com/l9F4mXq.png 


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#issuecomment-67120867
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24483/
    Test PASSed.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66854184
  
      [Test build #24420 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24420/consoleFull) for   PR 3672 at commit [`31a6520`](https://github.com/apache/spark/commit/31a6520aea59f12d1a6896db6079c5cea62659a3).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66936450
  
      [Test build #24444 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24444/consoleFull) for   PR 3672 at commit [`526315d`](https://github.com/apache/spark/commit/526315d0cd3dbf34f7e6b5a3ea6d5187bf96cccb).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class Analyzer(catalog: Catalog, registry: FunctionRegistry, caseSensitive: Boolean)`



---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#discussion_r21729186
  
    --- Diff: docs/sql-programming-guide.md ---
    @@ -938,6 +938,18 @@ Configuration of Hive is done by placing your `hive-site.xml` file in `conf/`.
     
     You may also use the beeline script that comes with Hive.
     
    +Thrift JDBC server also support sending thrift RPC messages over HTTP transport. 
    +Use the following setting to enable HTTP mode as system property or in `hive-site.xml` file in `conf/`: 
    +
    +    hive.server2.transport.mode - Set this to `http` 
    +    hive.server2.thrift.http.port - HTTP port number fo listen on; default is 10001
    +
    +To test, use beeline to connect to the JDBC/ODBC server in http mode with:
    +
    +    beeline> !connect jdbc:hive2://<host>:<port>/<database>?hive.server2.transport.mode=http;hive.server2.thrift.http.path=<http_endpoint>
    --- End diff --
    
    Same as above.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#discussion_r21728999
  
    --- Diff: docs/sql-programming-guide.md ---
    @@ -938,6 +938,18 @@ Configuration of Hive is done by placing your `hive-site.xml` file in `conf/`.
     
     You may also use the beeline script that comes with Hive.
     
    +Thrift JDBC server also support sending thrift RPC messages over HTTP transport. 
    +Use the following setting to enable HTTP mode as system property or in `hive-site.xml` file in `conf/`: 
    +
    +    hive.server2.transport.mode - Set this to `http` 
    +    hive.server2.thrift.http.port - HTTP port number fo listen on; default is 10001
    --- End diff --
    
    Failed to render the documentation locally because of my local configuration issues. Not sure whether this renders properly. Maybe we need to quote them with `highlight`:
    
    ```
    {% highlight bash %}
    ...
    {% endhighlight bash %}
    ```


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66916267
  
      [Test build #24440 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24440/consoleFull) for   PR 3672 at commit [`31a6520`](https://github.com/apache/spark/commit/31a6520aea59f12d1a6896db6079c5cea62659a3).
     * This patch merges cleanly.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#discussion_r21729137
  
    --- Diff: docs/sql-programming-guide.md ---
    @@ -938,6 +938,18 @@ Configuration of Hive is done by placing your `hive-site.xml` file in `conf/`.
     
     You may also use the beeline script that comes with Hive.
     
    +Thrift JDBC server also support sending thrift RPC messages over HTTP transport. 
    +Use the following setting to enable HTTP mode as system property or in `hive-site.xml` file in `conf/`: 
    +
    +    hive.server2.transport.mode - Set this to `http` 
    +    hive.server2.thrift.http.port - HTTP port number fo listen on; default is 10001
    --- End diff --
    
    is there instruction on how to render doc like this locally? I can give this a try.


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

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

    https://github.com/apache/spark/pull/3672#discussion_r21721702
  
    --- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
    @@ -113,7 +118,8 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
     
       def startThriftServer(
           port: Int,
    -      serverStartTimeout: FiniteDuration = 1.minute)(
    +      serverStartTimeout: FiniteDuration = 1.minute,
    +      httpMode: Boolean = false )(
    --- End diff --
    
    Done. 


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#issuecomment-66731363
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24389/
    Test PASSed.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#discussion_r21774497
  
    --- Diff: docs/sql-programming-guide.md ---
    @@ -938,6 +938,18 @@ Configuration of Hive is done by placing your `hive-site.xml` file in `conf/`.
     
     You may also use the beeline script that comes with Hive.
     
    +Thrift JDBC server also support sending thrift RPC messages over HTTP transport. 
    +Use the following setting to enable HTTP mode as system property or in `hive-site.xml` file in `conf/`: 
    +
    +    hive.server2.transport.mode - Set this to `http` 
    +    hive.server2.thrift.http.port - HTTP port number fo listen on; default is 10001
    --- End diff --
    
    Rendered the site and fixed few issues caused by ">" chars. 
    Final image can be found here: http://imgur.com/LPceQ9J



---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#discussion_r21728960
  
    --- Diff: docs/sql-programming-guide.md ---
    @@ -938,6 +938,18 @@ Configuration of Hive is done by placing your `hive-site.xml` file in `conf/`.
     
     You may also use the beeline script that comes with Hive.
     
    +Thrift JDBC server also support sending thrift RPC messages over HTTP transport. 
    --- End diff --
    
    support => supports


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

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


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66583664
  
    ok to test


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

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

    https://github.com/apache/spark/pull/3672#issuecomment-66590225
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24356/
    Test PASSed.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-67113736
  
      [Test build #24483 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24483/consoleFull) for   PR 3672 at commit [`526315d`](https://github.com/apache/spark/commit/526315d0cd3dbf34f7e6b5a3ea6d5187bf96cccb).
     * This patch merges cleanly.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#issuecomment-66726018
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24385/
    Test PASSed.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66933007
  
      [Test build #24444 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24444/consoleFull) for   PR 3672 at commit [`526315d`](https://github.com/apache/spark/commit/526315d0cd3dbf34f7e6b5a3ea6d5187bf96cccb).
     * This patch merges cleanly.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-67105417
  
      [Test build #24477 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24477/consoleFull) for   PR 3672 at commit [`526315d`](https://github.com/apache/spark/commit/526315d0cd3dbf34f7e6b5a3ea6d5187bf96cccb).
     * This patch **fails PySpark unit tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class Analyzer(catalog: Catalog, registry: FunctionRegistry, caseSensitive: Boolean)`



---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66843487
  
      [Test build #24420 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24420/consoleFull) for   PR 3672 at commit [`31a6520`](https://github.com/apache/spark/commit/31a6520aea59f12d1a6896db6079c5cea62659a3).
     * This patch merges cleanly.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by judynash <gi...@git.apache.org>.
Github user judynash commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-67220291
  
    Thanks Cheng and Josh for restarting the tests! What's the next step now the test has passed?


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66612787
  
    Could you please also add a section in the SQL programming guide page to introduce how to enable the HTTP mode?


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

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

    https://github.com/apache/spark/pull/3672#discussion_r21662926
  
    --- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
    @@ -70,11 +70,16 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
         port
       }
     
    -  def withJdbcStatement(serverStartTimeout: FiniteDuration = 1.minute)(f: Statement => Unit) {
    +  def withJdbcStatement(serverStartTimeout: FiniteDuration = 1.minute, httpMode: Boolean = false)(f: Statement => Unit) {
         val port = randomListeningPort
     
    -    startThriftServer(port, serverStartTimeout) {
    -      val jdbcUri = s"jdbc:hive2://${"localhost"}:$port/"
    +    startThriftServer(port, serverStartTimeout, httpMode) {
    +      val jdbcUri = if (httpMode) {
    +        s"jdbc:hive2://${"localhost"}:$port/default?hive.server2.transport.mode=http;hive.server2.thrift.http.path=cliservice"
    --- End diff --
    
    100 columns exceeded.


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

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

    https://github.com/apache/spark/pull/3672#discussion_r21663078
  
    --- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
    @@ -217,6 +236,25 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
         }
       }
     
    +  test("Test JDBC query execution in Http Mode") {
    +    withJdbcStatement( httpMode = true ) { statement =>
    --- End diff --
    
    Please remove spaces after `(` and before `)`.


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

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

    https://github.com/apache/spark/pull/3672#discussion_r21663058
  
    --- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
    @@ -121,15 +127,28 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
         val warehousePath = getTempFilePath("warehouse")
         val metastorePath = getTempFilePath("metastore")
         val metastoreJdbcUri = s"jdbc:derby:;databaseName=$metastorePath;create=true"
    +
         val command =
    -      s"""$startScript
    -         |  --master local
    -         |  --hiveconf hive.root.logger=INFO,console
    -         |  --hiveconf ${ConfVars.METASTORECONNECTURLKEY}=$metastoreJdbcUri
    -         |  --hiveconf ${ConfVars.METASTOREWAREHOUSE}=$warehousePath
    -         |  --hiveconf ${ConfVars.HIVE_SERVER2_THRIFT_BIND_HOST}=${"localhost"}
    -         |  --hiveconf ${ConfVars.HIVE_SERVER2_THRIFT_PORT}=$port
    -       """.stripMargin.split("\\s+").toSeq
    +      if (httpMode){
    +          s"""$startScript
    +           |  --master local
    +           |  --hiveconf hive.root.logger=INFO,console
    +           |  --hiveconf ${ConfVars.METASTORECONNECTURLKEY}=$metastoreJdbcUri
    +           |  --hiveconf ${ConfVars.METASTOREWAREHOUSE}=$warehousePath
    +           |  --hiveconf ${ConfVars.HIVE_SERVER2_THRIFT_BIND_HOST}=${"localhost"}
    +           |  --hiveconf ${ConfVars.HIVE_SERVER2_TRANSPORT_MODE}=${"http"}
    +           |  --hiveconf ${ConfVars.HIVE_SERVER2_THRIFT_HTTP_PORT}=$port
    +            """.stripMargin.split("\\s+").toSeq
    +      } else {
    +          s"""$startScript
    +             |  --master local
    +             |  --hiveconf hive.root.logger=INFO,console
    +             |  --hiveconf ${ConfVars.METASTORECONNECTURLKEY}=$metastoreJdbcUri
    +             |  --hiveconf ${ConfVars.METASTOREWAREHOUSE}=$warehousePath
    +             |  --hiveconf ${ConfVars.HIVE_SERVER2_THRIFT_BIND_HOST}=${"localhost"}
    --- End diff --
    
    Ah, I see, the original code uses a redundant `${...}` wrapper, please help removing this one :)


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by judynash <gi...@git.apache.org>.
Github user judynash commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-67288934
  
    Not a problem at all. Just wanted to know to see if I need to patch our own 1.2 release.
    Thank you Cheng, Josh, and Michael for the prompt actions to get the pull through!
    
    From: Cheng Lian [mailto:notifications@github.com]
    Sent: Tuesday, December 16, 2014 11:54 PM
    To: apache/spark
    Cc: Judy Nash
    Subject: Re: [spark] [SQL] SPARK-4700: Add HTTP protocol spark thrift server (#3672)
    
    
    @judynash<https://github.com/judynash> Unfortunately we've already cut 1.2.0 RC2 (which is now officially 1.2.0 release) before this PR was submitted, so it will be part of 1.2.1. And sorry for all the noises from Jenkins, we upgraded Jenkins recently and caused some unexpected issues (thanks to Josh who fixed them!).
    
    —
    Reply to this email directly or view it on GitHub<https://github.com/apache/spark/pull/3672#issuecomment-67287991>.



---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66726017
  
      [Test build #24385 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24385/consoleFull) for   PR 3672 at commit [`2b1d312`](https://github.com/apache/spark/commit/2b1d31218de092f88dd53bde784c57358ef116a9).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#discussion_r21729377
  
    --- Diff: docs/sql-programming-guide.md ---
    @@ -938,6 +938,18 @@ Configuration of Hive is done by placing your `hive-site.xml` file in `conf/`.
     
     You may also use the beeline script that comes with Hive.
     
    +Thrift JDBC server also support sending thrift RPC messages over HTTP transport. 
    +Use the following setting to enable HTTP mode as system property or in `hive-site.xml` file in `conf/`: 
    +
    +    hive.server2.transport.mode - Set this to `http` 
    +    hive.server2.thrift.http.port - HTTP port number fo listen on; default is 10001
    --- End diff --
    
    The documentation can be built with Jekyll. You need RubyGems installed, then
    
    1. `gem install jekyll` to install jekyll
    2. `cd` into `$SPARK_HOME/docs`
    3. `jekyll build`


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#issuecomment-66919317
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24440/
    Test FAILed.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-67113444
  
    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 pull request: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66584154
  
      [Test build #24356 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24356/consoleFull) for   PR 3672 at commit [`377532c`](https://github.com/apache/spark/commit/377532cdff819010aef1786f84c987eddb63af45).
     * This patch merges cleanly.


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

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

    https://github.com/apache/spark/pull/3672#discussion_r21721569
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -85,10 +86,22 @@ private[hive] class HiveThriftServer2(hiveContext: HiveContext)
         setSuperField(this, "cliService", sparkSqlCliService)
         addService(sparkSqlCliService)
     
    -    val thriftCliService = new ThriftBinaryCLIService(sparkSqlCliService)
    -    setSuperField(this, "thriftCLIService", thriftCliService)
    -    addService(thriftCliService)
    +    if (isHTTPTransportMode(hiveConf)){
    +      val thriftCliService = new ThriftHttpCLIService(sparkSqlCliService)
    +      setSuperField(this, "thriftCLIService", thriftCliService)
    +      addService(thriftCliService)
    +    } else {
    +      val thriftCliService = new ThriftBinaryCLIService(sparkSqlCliService)
    +      setSuperField(this, "thriftCLIService", thriftCliService)
    +      addService(thriftCliService)
    +    }
     
         initCompositeService(hiveConf)
       }
    +
    +  private def isHTTPTransportMode(hiveConf: HiveConf): Boolean = {
    +    val transportMode: String = hiveConf.getVar(ConfVars.HIVE_SERVER2_TRANSPORT_MODE)
    +    return transportMode.equalsIgnoreCase("http")
    --- End diff --
    
    Sounds good. Removed the return value. 


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by judynash <gi...@git.apache.org>.
Github user judynash commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66950036
  
    Thank you Cheng for the update.
    
    From: Cheng Lian [mailto:notifications@github.com]
    Sent: Sunday, December 14, 2014 6:57 PM
    To: apache/spark
    Cc: Judy Nash
    Subject: Re: [spark] [SQL] SPARK-4700: Add HTTP protocol spark thrift server (#3672)
    
    
    The most recent test failures should be caused by other components. Let's wait for a moment. This PR now LGTM.
    
    —
    Reply to this email directly or view it on GitHub<https://github.com/apache/spark/pull/3672#issuecomment-66944216>.



---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

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

    https://github.com/apache/spark/pull/3672#discussion_r21663114
  
    --- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
    @@ -267,6 +305,14 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
         }
       }
     
    +  test("Checks Hive version in Http Mode") {
    +    withJdbcStatement( httpMode = true ) { statement =>
    --- End diff --
    
    Remove the extra spaces.


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66590223
  
      [Test build #24356 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24356/consoleFull) for   PR 3672 at commit [`377532c`](https://github.com/apache/spark/commit/377532cdff819010aef1786f84c987eddb63af45).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66919314
  
      [Test build #24440 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24440/consoleFull) for   PR 3672 at commit [`31a6520`](https://github.com/apache/spark/commit/31a6520aea59f12d1a6896db6079c5cea62659a3).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class Analyzer(catalog: Catalog, registry: FunctionRegistry, caseSensitive: Boolean)`



---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

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

    https://github.com/apache/spark/pull/3672#discussion_r21718707
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -85,10 +86,22 @@ private[hive] class HiveThriftServer2(hiveContext: HiveContext)
         setSuperField(this, "cliService", sparkSqlCliService)
         addService(sparkSqlCliService)
     
    -    val thriftCliService = new ThriftBinaryCLIService(sparkSqlCliService)
    -    setSuperField(this, "thriftCLIService", thriftCliService)
    -    addService(thriftCliService)
    +    if (isHTTPTransportMode(hiveConf)){
    +      val thriftCliService = new ThriftHttpCLIService(sparkSqlCliService)
    +      setSuperField(this, "thriftCLIService", thriftCliService)
    +      addService(thriftCliService)
    +    } else {
    +      val thriftCliService = new ThriftBinaryCLIService(sparkSqlCliService)
    +      setSuperField(this, "thriftCLIService", thriftCliService)
    +      addService(thriftCliService)
    +    }
     
         initCompositeService(hiveConf)
       }
    +
    +  private def isHTTPTransportMode(hiveConf: HiveConf): Boolean = {
    +    val transportMode: String = hiveConf.getVar(ConfVars.HIVE_SERVER2_TRANSPORT_MODE)
    +    return transportMode.equalsIgnoreCase("http")
    --- End diff --
    
    Didn't know return value is not required (thanks for pointing that out). Style-wise, is it a good idea to omit return though? Explicitly use the return value seems more clear for code readability. If this is the preferred way in scala, I will remove the return value. 



---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-67227330
  
    LGTM, thanks for working on this!  Merging to master and 1.2.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#issuecomment-66936452
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24444/
    Test FAILed.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66721275
  
      [Test build #24385 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24385/consoleFull) for   PR 3672 at commit [`2b1d312`](https://github.com/apache/spark/commit/2b1d31218de092f88dd53bde784c57358ef116a9).
     * This patch merges cleanly.


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-66587346
  
    One more thing, please rename the PR title to "[SQL] SPARK-4700: ...". You can find names of all valid Spark components from the JIRA. (Couldn't provide a URL right now because JIRA is reindexing...)


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#issuecomment-66854192
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24420/
    Test FAILed.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-67222875
  
    I don't really know anything about the thriftserver, so I'm not comfortable merging this PR myself.  Therefore, let's ping @marmbrus for a final sign-off + commit.


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

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

    https://github.com/apache/spark/pull/3672#discussion_r21721601
  
    --- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
    @@ -70,11 +70,16 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
         port
       }
     
    -  def withJdbcStatement(serverStartTimeout: FiniteDuration = 1.minute)(f: Statement => Unit) {
    +  def withJdbcStatement(serverStartTimeout: FiniteDuration = 1.minute, httpMode: Boolean = false)(f: Statement => Unit) {
         val port = randomListeningPort
     
    -    startThriftServer(port, serverStartTimeout) {
    -      val jdbcUri = s"jdbc:hive2://${"localhost"}:$port/"
    +    startThriftServer(port, serverStartTimeout, httpMode) {
    +      val jdbcUri = if (httpMode) {
    +        s"jdbc:hive2://${"localhost"}:$port/default?hive.server2.transport.mode=http;hive.server2.thrift.http.path=cliservice"
    --- End diff --
    
    Fixed. 


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-67097621
  
    Jenkins, 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 pull request: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-67097904
  
      [Test build #24477 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24477/consoleFull) for   PR 3672 at commit [`526315d`](https://github.com/apache/spark/commit/526315d0cd3dbf34f7e6b5a3ea6d5187bf96cccb).
     * This patch merges cleanly.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/3672#issuecomment-67287991
  
    @judynash Unfortunately we've already cut 1.2.0 RC2 (which is now officially 1.2.0 release) before this PR was submitted, so it will be part of 1.2.1. And sorry for all the noises from Jenkins, we upgraded Jenkins recently and caused some unexpected issues (thanks to Josh who fixed them!).


---
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: [SparkSQL, Thrift] SPARK-4700: Add HTTP protoc...

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

    https://github.com/apache/spark/pull/3672#discussion_r21673560
  
    --- Diff: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2Suite.scala ---
    @@ -121,15 +127,28 @@ class HiveThriftServer2Suite extends FunSuite with Logging {
         val warehousePath = getTempFilePath("warehouse")
         val metastorePath = getTempFilePath("metastore")
         val metastoreJdbcUri = s"jdbc:derby:;databaseName=$metastorePath;create=true"
    +
         val command =
    -      s"""$startScript
    -         |  --master local
    -         |  --hiveconf hive.root.logger=INFO,console
    -         |  --hiveconf ${ConfVars.METASTORECONNECTURLKEY}=$metastoreJdbcUri
    -         |  --hiveconf ${ConfVars.METASTOREWAREHOUSE}=$warehousePath
    -         |  --hiveconf ${ConfVars.HIVE_SERVER2_THRIFT_BIND_HOST}=${"localhost"}
    -         |  --hiveconf ${ConfVars.HIVE_SERVER2_THRIFT_PORT}=$port
    -       """.stripMargin.split("\\s+").toSeq
    +      if (httpMode){
    --- End diff --
    
    A space before `{`


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#issuecomment-67105422
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24477/
    Test FAILed.


---
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: [SQL] SPARK-4700: Add HTTP protocol spark thri...

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

    https://github.com/apache/spark/pull/3672#discussion_r21729017
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -85,10 +86,22 @@ private[hive] class HiveThriftServer2(hiveContext: HiveContext)
         setSuperField(this, "cliService", sparkSqlCliService)
         addService(sparkSqlCliService)
     
    -    val thriftCliService = new ThriftBinaryCLIService(sparkSqlCliService)
    -    setSuperField(this, "thriftCLIService", thriftCliService)
    -    addService(thriftCliService)
    +    if (isHTTPTransportMode(hiveConf)){
    --- End diff --
    
    Add a space before `{`.


---
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