You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@gearpump.apache.org by Roshanson <gi...@git.apache.org> on 2016/09/22 05:04:53 UTC

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

GitHub user Roshanson opened a pull request:

    https://github.com/apache/incubator-gearpump/pull/86

    	[GEARPUMP-204]add unit test for external_hbase module

    	[GEARPUMP-204]add unit test for external_hbase module

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

    $ git pull https://github.com/Roshanson/incubator-gearpump fix-addHBaseUT

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

    https://github.com/apache/incubator-gearpump/pull/86.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 #86
    
----
commit bc5c21e52fcdc46f1f4b6b350d4351bc2e1134f7
Author: Roshanson <73...@qq.com>
Date:   2016-09-02T11:17:33Z

    refactor example sources task to use DataSourceAPI

commit 5db634a5f6cfa2ff198a46019bb8a4fa6b63cfeb
Author: 736781877@qq.com <doyouta123>
Date:   2016-09-02T12:26:48Z

    refactor example sources task to use DataSourceAPI

commit 57c7e8817ba9c8683039e050a217904d065f7a13
Author: 736781877@qq.com <doyouta123>
Date:   2016-09-02T12:31:28Z

    refactor example sources task to use DataSourceAPI

commit 0097e2527783114a7d8890eca7173e5fb55a6194
Author: Roshanson <73...@qq.com>
Date:   2016-09-13T13:30:57Z

    [GEARPUMP-204]Add unit test for external_hbase module

commit 99257d02e54bd0fa7ad530487aa1b2282964ca85
Author: roshanson <doyouta123>
Date:   2016-09-19T09:00:49Z

    connection to hbase

commit a14df0d10d03219800a6b1a166ec9d773cfa76b3
Author: roshanson <doyouta123>
Date:   2016-09-21T04:33:57Z

    [GEARPUMP-204]add unit test for external_hbase module

commit 8ef7dbb0d9b816a595441e66b0f71a041fc5b9f8
Author: roshanson <doyouta123>
Date:   2016-09-21T05:57:24Z

    [GEARPUMP-204]add unit test for external_hbase module

commit 87364c049fd283c5da6898abf61973eae031a8a7
Author: roshanson <doyouta123>
Date:   2016-09-22T03:36:25Z

    [GEARPUMP-204]add unit test for external_hbase module

----


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

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r79979033
  
    --- Diff: external/hbase/src/main/scala/org/apache/gearpump/external/hbase/HBaseSink.scala ---
    @@ -19,39 +19,47 @@ package org.apache.gearpump.external.hbase
     
     import java.io.{File, ObjectInputStream, ObjectOutputStream}
     
    +import org.apache.gearpump.Message
    +import org.apache.gearpump.cluster.UserConfig
    +import org.apache.gearpump.streaming.sink.DataSink
    +import org.apache.gearpump.streaming.task.TaskContext
    +import org.apache.gearpump.util.{Constants, FileUtils}
     import org.apache.hadoop.conf.Configuration
     import org.apache.hadoop.hbase.client.{Connection, ConnectionFactory, Put}
    +import org.apache.hadoop.hbase.security.UserProvider
     import org.apache.hadoop.hbase.util.Bytes
     import org.apache.hadoop.hbase.{HBaseConfiguration, TableName}
    -import org.apache.hadoop.hbase.security.{User, UserProvider}
     import org.apache.hadoop.security.UserGroupInformation
     
    -import org.apache.gearpump.Message
    -import org.apache.gearpump.cluster.UserConfig
    -import org.apache.gearpump.streaming.sink.DataSink
    -import org.apache.gearpump.streaming.task.TaskContext
    -import org.apache.gearpump.util.{Constants, FileUtils}
    +class HBaseSink(userconfig: UserConfig, tableName: String, @transient var connection: Connection,
    +    @transient var configuration: Configuration)
    +  extends DataSink {
     
    -class HBaseSink(
    -    userconfig: UserConfig, tableName: String, @transient var configuration: Configuration)
    -  extends DataSink{
    -  lazy val connection = HBaseSink.getConnection(userconfig, configuration)
    +
    +  // var connection = HBaseSink.getConnection(userconfig, configuration)
       lazy val table = connection.getTable(TableName.valueOf(tableName))
     
       override def open(context: TaskContext): Unit = {}
     
       def this(userconfig: UserConfig, tableName: String) = {
    -    this(userconfig, tableName, HBaseConfiguration.create())
    +    this(userconfig, tableName, HBaseSink.getConnection(userconfig, HBaseConfiguration.create()),
    +      HBaseConfiguration.create())
    +  }
    +  def this(userconfig: UserConfig, tableName: String, configuration: Configuration) = {
    +    this(userconfig, tableName, HBaseSink.getConnection(userconfig, HBaseConfiguration.create()),
    +      HBaseConfiguration.create())
    --- End diff --
    
    `configuration` is passed in 


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

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r80017362
  
    --- Diff: external/hbase/src/main/scala/org/apache/gearpump/external/hbase/HBaseSink.scala ---
    @@ -19,29 +19,33 @@ package org.apache.gearpump.external.hbase
     
     import java.io.{File, ObjectInputStream, ObjectOutputStream}
     
    +import org.apache.gearpump.Message
    +import org.apache.gearpump.cluster.UserConfig
    +import org.apache.gearpump.streaming.sink.DataSink
    +import org.apache.gearpump.streaming.task.TaskContext
    +import org.apache.gearpump.util.{Constants, FileUtils}
     import org.apache.hadoop.conf.Configuration
     import org.apache.hadoop.hbase.client.{Connection, ConnectionFactory, Put}
    +import org.apache.hadoop.hbase.security.UserProvider
     import org.apache.hadoop.hbase.util.Bytes
     import org.apache.hadoop.hbase.{HBaseConfiguration, TableName}
    -import org.apache.hadoop.hbase.security.{User, UserProvider}
     import org.apache.hadoop.security.UserGroupInformation
     
    -import org.apache.gearpump.Message
    -import org.apache.gearpump.cluster.UserConfig
    -import org.apache.gearpump.streaming.sink.DataSink
    -import org.apache.gearpump.streaming.task.TaskContext
    -import org.apache.gearpump.util.{Constants, FileUtils}
    +class HBaseSink(userconfig: UserConfig, tableName: String, @transient var connection: Connection,
    +    @transient var configuration: Configuration)
    +  extends DataSink {
     
    -class HBaseSink(
    -    userconfig: UserConfig, tableName: String, @transient var configuration: Configuration)
    -  extends DataSink{
    -  lazy val connection = HBaseSink.getConnection(userconfig, configuration)
       lazy val table = connection.getTable(TableName.valueOf(tableName))
     
       override def open(context: TaskContext): Unit = {}
     
       def this(userconfig: UserConfig, tableName: String) = {
    -    this(userconfig, tableName, HBaseConfiguration.create())
    +    this(userconfig, tableName, HBaseSink.getConnection(userconfig, HBaseConfiguration.create()),
    --- End diff --
    
    why create configuration twice 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.
---

[GitHub] incubator-gearpump issue #86: [GEARPUMP-204]add unit test for external_hbase...

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

    https://github.com/apache/incubator-gearpump/pull/86
  
    Generally my question is that is there any way implementing the HBaseSinkSpec without changing the HBaseSink?


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

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r79979075
  
    --- Diff: external/hbase/src/main/scala/org/apache/gearpump/external/hbase/HBaseSink.scala ---
    @@ -19,39 +19,47 @@ package org.apache.gearpump.external.hbase
     
     import java.io.{File, ObjectInputStream, ObjectOutputStream}
     
    +import org.apache.gearpump.Message
    +import org.apache.gearpump.cluster.UserConfig
    +import org.apache.gearpump.streaming.sink.DataSink
    +import org.apache.gearpump.streaming.task.TaskContext
    +import org.apache.gearpump.util.{Constants, FileUtils}
     import org.apache.hadoop.conf.Configuration
     import org.apache.hadoop.hbase.client.{Connection, ConnectionFactory, Put}
    +import org.apache.hadoop.hbase.security.UserProvider
     import org.apache.hadoop.hbase.util.Bytes
     import org.apache.hadoop.hbase.{HBaseConfiguration, TableName}
    -import org.apache.hadoop.hbase.security.{User, UserProvider}
     import org.apache.hadoop.security.UserGroupInformation
     
    -import org.apache.gearpump.Message
    -import org.apache.gearpump.cluster.UserConfig
    -import org.apache.gearpump.streaming.sink.DataSink
    -import org.apache.gearpump.streaming.task.TaskContext
    -import org.apache.gearpump.util.{Constants, FileUtils}
    +class HBaseSink(userconfig: UserConfig, tableName: String, @transient var connection: Connection,
    +    @transient var configuration: Configuration)
    +  extends DataSink {
     
    -class HBaseSink(
    -    userconfig: UserConfig, tableName: String, @transient var configuration: Configuration)
    -  extends DataSink{
    -  lazy val connection = HBaseSink.getConnection(userconfig, configuration)
    +
    +  // var connection = HBaseSink.getConnection(userconfig, configuration)
       lazy val table = connection.getTable(TableName.valueOf(tableName))
     
       override def open(context: TaskContext): Unit = {}
     
       def this(userconfig: UserConfig, tableName: String) = {
    -    this(userconfig, tableName, HBaseConfiguration.create())
    +    this(userconfig, tableName, HBaseSink.getConnection(userconfig, HBaseConfiguration.create()),
    +      HBaseConfiguration.create())
    +  }
    +  def this(userconfig: UserConfig, tableName: String, configuration: Configuration) = {
    +    this(userconfig, tableName, HBaseSink.getConnection(userconfig, HBaseConfiguration.create()),
    +      HBaseConfiguration.create())
       }
     
    +
    --- End diff --
    
    please remove these blank line changes


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

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r80175062
  
    --- Diff: external/hbase/src/main/scala/org/apache/gearpump/external/hbase/HBaseSink.scala ---
    @@ -115,15 +116,24 @@ object HBaseSink {
       val COLUMN_NAME = "hbase.table.column.name"
       val HBASE_USER = "hbase.user"
     
    -  def apply[T](userconfig: UserConfig, tableName: String): HBaseSink = {
    -    new HBaseSink(userconfig, tableName)
    -  }
     
       def apply[T](userconfig: UserConfig, tableName: String, configuration: Configuration)
    -    : HBaseSink = {
    +  : HBaseSink = {
         new HBaseSink(userconfig, tableName, configuration)
       }
     
    +  def apply[T](
    --- End diff --
    
    Code format


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

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r80017412
  
    --- Diff: external/hbase/src/main/scala/org/apache/gearpump/external/hbase/HBaseSink.scala ---
    @@ -97,7 +101,7 @@ class HBaseSink(
       }
     
       /**
    -   * Overrides Java's default deserialization
    +   * Overrides Java's default serialization
    --- End diff --
    
    serialization ?


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

[GitHub] incubator-gearpump issue #86: [GEARPUMP-204]add unit test for external_hbase...

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

    https://github.com/apache/incubator-gearpump/pull/86
  
    @huafengw the current `HBaseSink` constructor is not testable. I'm afraid changing it is the only way to go.


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

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r79979402
  
    --- Diff: external/hbase/src/main/scala/org/apache/gearpump/external/hbase/HBaseSink.scala ---
    @@ -148,11 +158,24 @@ object HBaseSink {
         val userName = userConfig.getString(HBASE_USER)
         if (userName.isEmpty) {
           ConnectionFactory.createConnection(configuration)
    +
         } else {
           val user = UserProvider.instantiate(configuration)
             .create(UserGroupInformation.createRemoteUser(userName.get))
           ConnectionFactory.createConnection(configuration, user)
         }
     
    +
    +  }
    +  def getConn(connection: Connection, isTest: Boolean, userConfig: UserConfig,
    --- End diff --
    
    We don't need `isTest` here since this method is not supposed to be called in 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.
---

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r79979330
  
    --- Diff: external/hbase/src/main/scala/org/apache/gearpump/external/hbase/HBaseSink.scala ---
    @@ -19,39 +19,47 @@ package org.apache.gearpump.external.hbase
     
     import java.io.{File, ObjectInputStream, ObjectOutputStream}
     
    +import org.apache.gearpump.Message
    +import org.apache.gearpump.cluster.UserConfig
    +import org.apache.gearpump.streaming.sink.DataSink
    +import org.apache.gearpump.streaming.task.TaskContext
    +import org.apache.gearpump.util.{Constants, FileUtils}
     import org.apache.hadoop.conf.Configuration
     import org.apache.hadoop.hbase.client.{Connection, ConnectionFactory, Put}
    +import org.apache.hadoop.hbase.security.UserProvider
     import org.apache.hadoop.hbase.util.Bytes
     import org.apache.hadoop.hbase.{HBaseConfiguration, TableName}
    -import org.apache.hadoop.hbase.security.{User, UserProvider}
     import org.apache.hadoop.security.UserGroupInformation
     
    -import org.apache.gearpump.Message
    -import org.apache.gearpump.cluster.UserConfig
    -import org.apache.gearpump.streaming.sink.DataSink
    -import org.apache.gearpump.streaming.task.TaskContext
    -import org.apache.gearpump.util.{Constants, FileUtils}
    +class HBaseSink(userconfig: UserConfig, tableName: String, @transient var connection: Connection,
    +    @transient var configuration: Configuration)
    +  extends DataSink {
     
    -class HBaseSink(
    -    userconfig: UserConfig, tableName: String, @transient var configuration: Configuration)
    -  extends DataSink{
    -  lazy val connection = HBaseSink.getConnection(userconfig, configuration)
    +
    +  // var connection = HBaseSink.getConnection(userconfig, configuration)
    --- End diff --
    
    to allow it be mocked in 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.
---

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r80688415
  
    --- Diff: external/hbase/src/main/scala/org/apache/gearpump/external/hbase/HBaseSink.scala ---
    @@ -19,27 +19,33 @@ package org.apache.gearpump.external.hbase
     
     import java.io.{File, ObjectInputStream, ObjectOutputStream}
     
    +import org.apache.gearpump.Message
    +import org.apache.gearpump.cluster.UserConfig
    +import org.apache.gearpump.streaming.sink.DataSink
    +import org.apache.gearpump.streaming.task.TaskContext
    +import org.apache.gearpump.util.{Constants, FileUtils}
     import org.apache.hadoop.conf.Configuration
     import org.apache.hadoop.hbase.client.{Connection, ConnectionFactory, Put}
    +import org.apache.hadoop.hbase.security.UserProvider
     import org.apache.hadoop.hbase.util.Bytes
     import org.apache.hadoop.hbase.{HBaseConfiguration, TableName}
    -import org.apache.hadoop.hbase.security.{User, UserProvider}
     import org.apache.hadoop.security.UserGroupInformation
     
    -import org.apache.gearpump.Message
    -import org.apache.gearpump.cluster.UserConfig
    -import org.apache.gearpump.streaming.sink.DataSink
    -import org.apache.gearpump.streaming.task.TaskContext
    -import org.apache.gearpump.util.{Constants, FileUtils}
    +class HBaseSink(userconfig: UserConfig, tableName: String,
    +    val conn: (UserConfig, Configuration)
    +    => Connection, @transient var configuration: Configuration)
    +  extends DataSink {
     
    -class HBaseSink(
    -    userconfig: UserConfig, tableName: String, @transient var configuration: Configuration)
    -  extends DataSink{
    -  lazy val connection = HBaseSink.getConnection(userconfig, configuration)
    +  lazy val connection = conn(userconfig, configuration)
       lazy val table = connection.getTable(TableName.valueOf(tableName))
     
       override def open(context: TaskContext): Unit = {}
     
    +  def this(userconfig: UserConfig, tableName: String, configuration: Configuration) = {
    +    this(userconfig, tableName, (userconfig: UserConfig, config: Configuration) =>
    +      {HBaseSink.getConnection(userconfig, config)}, configuration)
    --- End diff --
    
    just passing in `getConnection` will do, I think


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

[GitHub] incubator-gearpump issue #86: [GEARPUMP-204]add unit test for external_hbase...

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

    https://github.com/apache/incubator-gearpump/pull/86
  
    ## [Current coverage](https://codecov.io/gh/apache/incubator-gearpump/pull/86?src=pr) is 70.63% (diff: 22.22%)
    > Merging [#86](https://codecov.io/gh/apache/incubator-gearpump/pull/86?src=pr) into [master](https://codecov.io/gh/apache/incubator-gearpump/branch/master?src=pr) will decrease coverage by **0.17%**
    
    
    ```diff
    @@             master        #86   diff @@
    ==========================================
      Files           178        178          
      Lines          5903       5909     +6   
      Methods        5584       5403   -181   
      Messages          0          0          
      Branches        319        506   +187   
    ==========================================
    - Hits           4180       4174     -6   
    - Misses         1723       1735    +12   
      Partials          0          0          
    ```
    
    ![Sunburst](https://codecov.io/gh/apache/incubator-gearpump/pull/86/graphs/sunburst.svg?src=pr&size=150)
    
    > Powered by [Codecov](https://codecov.io?src=pr). Last update [5cd5b93...3086e45](https://codecov.io/gh/apache/incubator-gearpump/compare/5cd5b9304f0f703c6f89d2865e5bb7adbb631c74...3086e4535fbbaa1c6e652f8e3cc3fdeb7bf24ead?src=pr)


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

[GitHub] incubator-gearpump issue #86: [GEARPUMP-204]add unit test for external_hbase...

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

    https://github.com/apache/incubator-gearpump/pull/86
  
    @huafengw any more comments ?


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

[GitHub] incubator-gearpump issue #86: [GEARPUMP-204]add unit test for external_hbase...

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

    https://github.com/apache/incubator-gearpump/pull/86
  
    +1


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

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r79979548
  
    --- Diff: external/hbase/src/test/scala/org/apache/gearpump/external/hbase/HBaseSinkSpec.scala ---
    @@ -17,24 +17,52 @@
      */
     package org.apache.gearpump.external.hbase
     
    +import org.apache.gearpump.cluster.UserConfig
    +import org.apache.gearpump.streaming.MockUtil
    +import org.apache.gearpump.streaming.task.TaskContext
    +import org.apache.hadoop.conf.Configuration
    +import org.apache.hadoop.hbase.TableName
    +import org.apache.hadoop.hbase.client._
    +import org.apache.hadoop.hbase.util.Bytes
    +import org.mockito.Mockito._
    +import org.scalatest.mock.MockitoSugar
     import org.scalatest.prop.PropertyChecks
     import org.scalatest.{Matchers, PropSpec}
     
    -class HBaseSinkSpec extends PropSpec with PropertyChecks with Matchers {
    +class HBaseSinkSpec extends PropSpec with PropertyChecks with Matchers with MockitoSugar {
     
       property("HBaseSink should insert a row successfully") {
     
    -  //  import Mockito._
    -  //  val htable = Mockito.mock(classOf[HTable])
    -  //  val row = "row"
    -  //  val group = "group"
    -  //  val name = "name"
    -  //  val value = "1.2"
    -  //  val put = new Put(Bytes.toBytes(row))
    -  //  put.add(Bytes.toBytes(group), Bytes.toBytes(name), Bytes.toBytes(value))
    -  //  val hbaseSink = HBaseSink(htable)
    -  //  hbaseSink.insert(put)
    -  //  verify(htable).put(put)
    +    val table = mock[Table]
    +    val config = mock[Configuration]
    +    val conn = mock[Connection]
    +    val taskContext = mock[TaskContext]
    +
    +
    +    val map = Map[String, String]("HBASESINK" -> "hbasesink", "TABLE_NAME" -> "hbase.table.name",
    +      "COLUMN_FAMILY" -> "hbase.table.column.family", "COLUMN_NAME" -> "hbase.table.column.name",
    +      "HBASE_USER" -> "hbase.user", "GEARPUMP_KERBEROS_PRINCIPAL" -> "gearpump.kerberos.principal",
    +      "GEARPUMP_KEYTAB_FILE" -> "gearpump.keytab.file"
    +    )
    +    val userconfig = new UserConfig(map)
    +    val tablename = "hbase"
    +    val row = "row"
    +    val group = "group"
    +    val name = "name"
    +    val value = "1.2"
    +
    +    when(conn.getTable(TableName.valueOf(tablename))).thenReturn(table)
    +
    +    val connection = HBaseSink.getConn(conn, true, userconfig, config)
    --- End diff --
    
    simply use mocked `conn` 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.
---

[GitHub] incubator-gearpump issue #86: [GEARPUMP-204]add unit test for external_hbase...

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

    https://github.com/apache/incubator-gearpump/pull/86
  
    One question, now the `connection` is a transient value, will there be a NPE after the HBaseSink deserialized?


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

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r79978998
  
    --- Diff: external/hbase/src/main/scala/org/apache/gearpump/external/hbase/HBaseSink.scala ---
    @@ -19,39 +19,47 @@ package org.apache.gearpump.external.hbase
     
     import java.io.{File, ObjectInputStream, ObjectOutputStream}
     
    +import org.apache.gearpump.Message
    +import org.apache.gearpump.cluster.UserConfig
    +import org.apache.gearpump.streaming.sink.DataSink
    +import org.apache.gearpump.streaming.task.TaskContext
    +import org.apache.gearpump.util.{Constants, FileUtils}
     import org.apache.hadoop.conf.Configuration
     import org.apache.hadoop.hbase.client.{Connection, ConnectionFactory, Put}
    +import org.apache.hadoop.hbase.security.UserProvider
     import org.apache.hadoop.hbase.util.Bytes
     import org.apache.hadoop.hbase.{HBaseConfiguration, TableName}
    -import org.apache.hadoop.hbase.security.{User, UserProvider}
     import org.apache.hadoop.security.UserGroupInformation
     
    -import org.apache.gearpump.Message
    -import org.apache.gearpump.cluster.UserConfig
    -import org.apache.gearpump.streaming.sink.DataSink
    -import org.apache.gearpump.streaming.task.TaskContext
    -import org.apache.gearpump.util.{Constants, FileUtils}
    +class HBaseSink(userconfig: UserConfig, tableName: String, @transient var connection: Connection,
    +    @transient var configuration: Configuration)
    +  extends DataSink {
     
    -class HBaseSink(
    -    userconfig: UserConfig, tableName: String, @transient var configuration: Configuration)
    -  extends DataSink{
    -  lazy val connection = HBaseSink.getConnection(userconfig, configuration)
    +
    +  // var connection = HBaseSink.getConnection(userconfig, configuration)
    --- End diff --
    
    I use  the parameter of  'connection'  to  separate real connection to HBase testing and unit testing.


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

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r79978331
  
    --- Diff: external/hbase/src/main/scala/org/apache/gearpump/external/hbase/HBaseSink.scala ---
    @@ -19,39 +19,47 @@ package org.apache.gearpump.external.hbase
     
     import java.io.{File, ObjectInputStream, ObjectOutputStream}
     
    +import org.apache.gearpump.Message
    +import org.apache.gearpump.cluster.UserConfig
    +import org.apache.gearpump.streaming.sink.DataSink
    +import org.apache.gearpump.streaming.task.TaskContext
    +import org.apache.gearpump.util.{Constants, FileUtils}
     import org.apache.hadoop.conf.Configuration
     import org.apache.hadoop.hbase.client.{Connection, ConnectionFactory, Put}
    +import org.apache.hadoop.hbase.security.UserProvider
     import org.apache.hadoop.hbase.util.Bytes
     import org.apache.hadoop.hbase.{HBaseConfiguration, TableName}
    -import org.apache.hadoop.hbase.security.{User, UserProvider}
     import org.apache.hadoop.security.UserGroupInformation
     
    -import org.apache.gearpump.Message
    -import org.apache.gearpump.cluster.UserConfig
    -import org.apache.gearpump.streaming.sink.DataSink
    -import org.apache.gearpump.streaming.task.TaskContext
    -import org.apache.gearpump.util.{Constants, FileUtils}
    +class HBaseSink(userconfig: UserConfig, tableName: String, @transient var connection: Connection,
    +    @transient var configuration: Configuration)
    +  extends DataSink {
     
    -class HBaseSink(
    -    userconfig: UserConfig, tableName: String, @transient var configuration: Configuration)
    -  extends DataSink{
    -  lazy val connection = HBaseSink.getConnection(userconfig, configuration)
    +
    +  // var connection = HBaseSink.getConnection(userconfig, configuration)
    --- End diff --
    
    Why change the `connection`?


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

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r79979601
  
    --- Diff: project/BuildExample.scala ---
    @@ -57,6 +57,7 @@ object BuildExample extends sbt.Build {
           )
       ) dependsOn(streaming % "test->test; provided", daemon % "test->test; provided")
     
    +
    --- End diff --
    
    unnecessary changes


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

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r79979272
  
    --- Diff: external/hbase/src/main/scala/org/apache/gearpump/external/hbase/HBaseSink.scala ---
    @@ -148,11 +158,24 @@ object HBaseSink {
         val userName = userConfig.getString(HBASE_USER)
         if (userName.isEmpty) {
           ConnectionFactory.createConnection(configuration)
    +
         } else {
           val user = UserProvider.instantiate(configuration)
             .create(UserGroupInformation.createRemoteUser(userName.get))
           ConnectionFactory.createConnection(configuration, user)
         }
     
    +
    +  }
    --- End diff --
    
    need a blank line 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.
---

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r79979177
  
    --- Diff: external/hbase/src/main/scala/org/apache/gearpump/external/hbase/HBaseSink.scala ---
    @@ -115,15 +116,24 @@ object HBaseSink {
       val COLUMN_NAME = "hbase.table.column.name"
       val HBASE_USER = "hbase.user"
     
    -  def apply[T](userconfig: UserConfig, tableName: String): HBaseSink = {
    -    new HBaseSink(userconfig, tableName)
    -  }
     
       def apply[T](userconfig: UserConfig, tableName: String, configuration: Configuration)
    -    : HBaseSink = {
    +  : HBaseSink = {
         new HBaseSink(userconfig, tableName, configuration)
       }
     
    +  def apply[T](
    --- End diff --
    
    this change is not necessary.


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

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r80017343
  
    --- Diff: external/hbase/src/main/scala/org/apache/gearpump/external/hbase/HBaseSink.scala ---
    @@ -19,29 +19,33 @@ package org.apache.gearpump.external.hbase
     
     import java.io.{File, ObjectInputStream, ObjectOutputStream}
     
    +import org.apache.gearpump.Message
    +import org.apache.gearpump.cluster.UserConfig
    +import org.apache.gearpump.streaming.sink.DataSink
    +import org.apache.gearpump.streaming.task.TaskContext
    +import org.apache.gearpump.util.{Constants, FileUtils}
     import org.apache.hadoop.conf.Configuration
     import org.apache.hadoop.hbase.client.{Connection, ConnectionFactory, Put}
    +import org.apache.hadoop.hbase.security.UserProvider
     import org.apache.hadoop.hbase.util.Bytes
     import org.apache.hadoop.hbase.{HBaseConfiguration, TableName}
    -import org.apache.hadoop.hbase.security.{User, UserProvider}
     import org.apache.hadoop.security.UserGroupInformation
     
    -import org.apache.gearpump.Message
    -import org.apache.gearpump.cluster.UserConfig
    -import org.apache.gearpump.streaming.sink.DataSink
    -import org.apache.gearpump.streaming.task.TaskContext
    -import org.apache.gearpump.util.{Constants, FileUtils}
    +class HBaseSink(userconfig: UserConfig, tableName: String, @transient var connection: Connection,
    +    @transient var configuration: Configuration)
    +  extends DataSink {
     
    -class HBaseSink(
    -    userconfig: UserConfig, tableName: String, @transient var configuration: Configuration)
    -  extends DataSink{
    -  lazy val connection = HBaseSink.getConnection(userconfig, configuration)
       lazy val table = connection.getTable(TableName.valueOf(tableName))
     
       override def open(context: TaskContext): Unit = {}
     
       def this(userconfig: UserConfig, tableName: String) = {
    -    this(userconfig, tableName, HBaseConfiguration.create())
    +    this(userconfig, tableName, HBaseSink.getConnection(userconfig, HBaseConfiguration.create()),
    +      HBaseConfiguration.create())
    +  }
    +  def this(userconfig: UserConfig, tableName: String, configuration: Configuration) = {
    +    this(userconfig, tableName, HBaseSink.getConnection(userconfig, HBaseConfiguration.create()),
    --- End diff --
    
    why create `configuration` here which is already passed in ?


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

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86


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

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r80167763
  
    --- Diff: external/hbase/src/main/scala/org/apache/gearpump/external/hbase/HBaseSink.scala ---
    @@ -114,16 +118,19 @@ object HBaseSink {
       val COLUMN_FAMILY = "hbase.table.column.family"
       val COLUMN_NAME = "hbase.table.column.name"
       val HBASE_USER = "hbase.user"
    -
    -  def apply[T](userconfig: UserConfig, tableName: String): HBaseSink = {
    -    new HBaseSink(userconfig, tableName)
    -  }
    +  private val CONFIG = HBaseConfiguration.create()
    --- End diff --
    
    will configuration be created at client side ? 


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

[GitHub] incubator-gearpump pull request #86: [GEARPUMP-204]add unit test for externa...

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

    https://github.com/apache/incubator-gearpump/pull/86#discussion_r79979616
  
    --- Diff: project/BuildExample.scala ---
    @@ -57,6 +57,7 @@ object BuildExample extends sbt.Build {
           )
       ) dependsOn(streaming % "test->test; provided", daemon % "test->test; provided")
     
    +
    --- End diff --
    
    unnecessary changes


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