You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@bahir.apache.org by ckadner <gi...@git.apache.org> on 2016/10/08 01:11:01 UTC

[GitHub] bahir pull request #24: [BAHIR-64] add Akka streaming test (send/receive)

GitHub user ckadner opened a pull request:

    https://github.com/apache/bahir/pull/24

    [BAHIR-64] add Akka streaming test (send/receive)

    [BAHIR-64: Add test that Akka streaming connector can receive data](https://issues.apache.org/jira/browse/BAHIR-64)
    
    This PR adds the test suite  ```AkkaStreamSuite.scala``` to the streaming connector *streaming-akka* to test data being sent and received.
    
    This code was originally contributed to the [spark-packages/dstream-akka](https://github.com/spark-packages/dstream-akka) project by [Shixiong Zhu](https://github.com/zsxwing) on March 18, 2016 (commit [19c9a9e](https://github.com/spark-packages/dstream-akka/commit/19c9a9e136f75a5ceb53bee0172b5719c6e7e626))

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

    $ git pull https://github.com/ckadner/bahir BAHIR-64_Akka_streaming_tests

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

    https://github.com/apache/bahir/pull/24.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 #24
    
----
commit 416905d98e8ce9954f6fa4005807d59694a02411
Author: Christian Kadner <ck...@us.ibm.com>
Date:   2016-09-28T19:41:35Z

    [BAHIR-64] add Akka streaming test (send/receive)

----


---
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] bahir pull request #24: [BAHIR-64] add Akka streaming test (send/receive)

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

    https://github.com/apache/bahir/pull/24#discussion_r86030626
  
    --- Diff: NOTICE ---
    @@ -2,4 +2,17 @@ Apache Bahir
     Copyright (c) 2016 The Apache Software Foundation.
     
     This product includes software developed at
    -The Apache Software Foundation (http://www.apache.org/).
    \ No newline at end of file
    +The Apache Software Foundation (http://www.apache.org/).
    +
    +
    +===================================================================
    --- End diff --
    
    Maybe this comment should just go in the header of the class.


---
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] bahir pull request #24: [BAHIR-64] add Akka streaming test (send/receive)

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

    https://github.com/apache/bahir/pull/24#discussion_r83519004
  
    --- Diff: NOTICE ---
    @@ -2,4 +2,17 @@ Apache Bahir
     Copyright (c) 2016 The Apache Software Foundation.
     
     This product includes software developed at
    -The Apache Software Foundation (http://www.apache.org/).
    \ No newline at end of file
    +The Apache Software Foundation (http://www.apache.org/).
    +
    +
    +===================================================================
    --- End diff --
    
    NOTICE files are not for this info. The information is always available on the git history.


---
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] bahir issue #24: [BAHIR-64] add Akka streaming test (send/receive)

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

    https://github.com/apache/bahir/pull/24
  
    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.
---

[GitHub] bahir pull request #24: [BAHIR-64] add Akka streaming test (send/receive)

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

    https://github.com/apache/bahir/pull/24#discussion_r86248567
  
    --- Diff: streaming-akka/src/main/scala/org/apache/spark/streaming/akka/ActorReceiver.scala ---
    @@ -65,6 +65,7 @@ object ActorReceiver {
         val akkaConf = ConfigFactory.parseString(
           s"""akka.actor.provider = "akka.remote.RemoteActorRefProvider"
              |akka.remote.enabled-transports = ["akka.remote.netty.tcp"]
    +         |akka.remote.netty.tcp.port = "0"
    --- End diff --
    
    we set it to `"0"` to have the port chosen automatically for the _Feeder_ actor and the _Receiver_ actor will "pick it up" when it subscribes to the _Feeder_ actor [Akka Remoting docs](http://doc.akka.io/docs/akka/2.3.11/scala/remoting.html#Preparing_your_ActorSystem_for_Remoting)


---
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] bahir pull request #24: [BAHIR-64] add Akka streaming test (send/receive)

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

    https://github.com/apache/bahir/pull/24#discussion_r86248560
  
    --- Diff: NOTICE ---
    @@ -2,4 +2,17 @@ Apache Bahir
     Copyright (c) 2016 The Apache Software Foundation.
     
     This product includes software developed at
    -The Apache Software Foundation (http://www.apache.org/).
    \ No newline at end of file
    +The Apache Software Foundation (http://www.apache.org/).
    +
    +
    +===================================================================
    --- End diff --
    
    Git history would show me as the contributor, not Shixiong, but the  [spark-packages](https://github.com/spark-packages) project has been deleted so I will revert the change to the NOTICE file


---
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] bahir issue #24: [BAHIR-64] add Akka streaming test (send/receive)

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

    https://github.com/apache/bahir/pull/24
  
    @lresende I had pushed an [update](https://github.com/apache/bahir/pull/24/commits/dce5c0eb7583153dfd8400573230ded693746f22) to address your comments last week. Could you review and merge this PR?
    
    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.
---

[GitHub] bahir pull request #24: [BAHIR-64] add Akka streaming test (send/receive)

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

    https://github.com/apache/bahir/pull/24


---
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] bahir pull request #24: [BAHIR-64] add Akka streaming test (send/receive)

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

    https://github.com/apache/bahir/pull/24#discussion_r83520719
  
    --- Diff: streaming-akka/src/main/scala/org/apache/spark/streaming/akka/ActorReceiver.scala ---
    @@ -65,6 +65,7 @@ object ActorReceiver {
         val akkaConf = ConfigFactory.parseString(
           s"""akka.actor.provider = "akka.remote.RemoteActorRefProvider"
              |akka.remote.enabled-transports = ["akka.remote.netty.tcp"]
    +         |akka.remote.netty.tcp.port = "0"
    --- End diff --
    
    Why are setting this to any available port ? Don't they need to match between the client and 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.
---