New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BAHIR-97] Akka as a streaming source for SQL Streaming. #38

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@sbcd90
Contributor

sbcd90 commented Mar 27, 2017

This PR is created to propose the addition of Akka compatible streaming source for Spark SQL Streaming.

@ApacheBahir

This comment has been minimized.

Show comment
Hide comment
@ApacheBahir

ApacheBahir Mar 27, 2017

Can one of the admins verify this patch?

ApacheBahir commented Mar 27, 2017

Can one of the admins verify this patch?

@ktoso

This comment has been minimized.

Show comment
Hide comment
@ktoso

ktoso Mar 27, 2017

Hi there, interesting patch :)
May I interest you with Akka Streams though? I think it would be the natural fit for the Akka side here. You could implement what you made Actors here as a GraphStage ( http://doc.akka.io/docs/akka/2.4.17/scala/stream/stream-customize.html ), and our blog series about how to write custom connectors ( http://blog.akka.io/integrations/2016/09/05/flow-control-at-the-akka-stream-boundary ).

Hope this helps! Would be super exciting to have a connection to Spark SQL Streaming directly exposed as Akka Source/Sink :-)

ktoso commented Mar 27, 2017

Hi there, interesting patch :)
May I interest you with Akka Streams though? I think it would be the natural fit for the Akka side here. You could implement what you made Actors here as a GraphStage ( http://doc.akka.io/docs/akka/2.4.17/scala/stream/stream-customize.html ), and our blog series about how to write custom connectors ( http://blog.akka.io/integrations/2016/09/05/flow-control-at-the-akka-stream-boundary ).

Hope this helps! Would be super exciting to have a connection to Spark SQL Streaming directly exposed as Akka Source/Sink :-)

@lresende

This comment has been minimized.

Show comment
Hide comment
@lresende

lresende Mar 28, 2017

Member

Please test this Jenkins

Member

lresende commented Mar 28, 2017

Please test this Jenkins

@ApacheBahir

This comment has been minimized.

Show comment
Hide comment
@ApacheBahir

ApacheBahir Mar 28, 2017

Build successful

ApacheBahir commented Mar 28, 2017

Build successful

@ApacheBahir

This comment has been minimized.

Show comment
Hide comment

ApacheBahir commented Mar 28, 2017

@ckadner

This comment has been minimized.

Show comment
Hide comment
@ckadner

ckadner Mar 28, 2017

Member

Note, our Jenkins build server does not currently run Scalatests (BAHIR-98) ...

17:20:55 No tests were executed.

17:20:54 [INFO] 
17:20:54 [INFO] --- maven-surefire-plugin:2.19.1:test (default-test) @ spark-sql-streaming-akka_2.11 ---
17:20:54 
17:20:54 -------------------------------------------------------
17:20:54  T E S T S
17:20:54 -------------------------------------------------------
17:20:54 OpenJDK 64-Bit Server VM warning: ignoring option MaxPermSize=512m; support was removed in 8.0
17:20:54 
17:20:54 Results :
17:20:54 
17:20:54 Tests run: 0, Failures: 0, Errors: 0, Skipped: 0
17:20:54 
17:20:54 [INFO] 
17:20:54 [INFO] --- maven-surefire-plugin:2.19.1:test (test) @ spark-sql-streaming-akka_2.11 ---
17:20:54 [INFO] Skipping execution of surefire because it has already been run for this configuration
17:20:54 [INFO] 
17:20:54 [INFO] --- scalatest-maven-plugin:1.0:test (test) @ spark-sql-streaming-akka_2.11 ---
17:20:54 OpenJDK 64-Bit Server VM warning: ignoring option MaxPermSize=512m; support was removed in 8.0
17:20:55 Discovery starting.
17:20:55 Discovery completed in 36 milliseconds.
17:20:55 Run starting. Expected test count is: 0
17:20:55 DiscoverySuite:
17:20:55 Run completed in 100 milliseconds.
17:20:55 Total number of tests run: 0
17:20:55 Suites: completed 1, aborted 0
17:20:55 Tests: succeeded 0, failed 0, canceled 0, ignored 0, pending 0
17:20:55 No tests were executed.
17:20:55 [INFO]
Member

ckadner commented Mar 28, 2017

Note, our Jenkins build server does not currently run Scalatests (BAHIR-98) ...

17:20:55 No tests were executed.

17:20:54 [INFO] 
17:20:54 [INFO] --- maven-surefire-plugin:2.19.1:test (default-test) @ spark-sql-streaming-akka_2.11 ---
17:20:54 
17:20:54 -------------------------------------------------------
17:20:54  T E S T S
17:20:54 -------------------------------------------------------
17:20:54 OpenJDK 64-Bit Server VM warning: ignoring option MaxPermSize=512m; support was removed in 8.0
17:20:54 
17:20:54 Results :
17:20:54 
17:20:54 Tests run: 0, Failures: 0, Errors: 0, Skipped: 0
17:20:54 
17:20:54 [INFO] 
17:20:54 [INFO] --- maven-surefire-plugin:2.19.1:test (test) @ spark-sql-streaming-akka_2.11 ---
17:20:54 [INFO] Skipping execution of surefire because it has already been run for this configuration
17:20:54 [INFO] 
17:20:54 [INFO] --- scalatest-maven-plugin:1.0:test (test) @ spark-sql-streaming-akka_2.11 ---
17:20:54 OpenJDK 64-Bit Server VM warning: ignoring option MaxPermSize=512m; support was removed in 8.0
17:20:55 Discovery starting.
17:20:55 Discovery completed in 36 milliseconds.
17:20:55 Run starting. Expected test count is: 0
17:20:55 DiscoverySuite:
17:20:55 Run completed in 100 milliseconds.
17:20:55 Total number of tests run: 0
17:20:55 Suites: completed 1, aborted 0
17:20:55 Tests: succeeded 0, failed 0, canceled 0, ignored 0, pending 0
17:20:55 No tests were executed.
17:20:55 [INFO]
@sbcd90

This comment has been minimized.

Show comment
Hide comment
@sbcd90

sbcd90 Mar 28, 2017

Contributor

Hello @ckadner ,
Are any actions need to be taken from my side? Like provide Java tests(Junits) instead of scala?

Contributor

sbcd90 commented Mar 28, 2017

Hello @ckadner ,
Are any actions need to be taken from my side? Like provide Java tests(Junits) instead of scala?

@lresende

This comment has been minimized.

Show comment
Hide comment
@lresende

lresende Mar 28, 2017

Member

retest this please

Member

lresende commented Mar 28, 2017

retest this please

@ckadner

This comment has been minimized.

Show comment
Hide comment
@ckadner

ckadner Mar 28, 2017

Member

@sbcd90 -- Scalatests should be sufficient. We need to fix our Jenkins integration test setup (BAHIR-98). Not an action item for you :-)

Member

ckadner commented Mar 28, 2017

@sbcd90 -- Scalatests should be sufficient. We need to fix our Jenkins integration test setup (BAHIR-98). Not an action item for you :-)

@ckadner

This comment has been minimized.

Show comment
Hide comment
@ckadner

ckadner Mar 28, 2017

Member

retest this please

Member

ckadner commented Mar 28, 2017

retest this please

@ApacheBahir

This comment has been minimized.

Show comment
Hide comment
@ApacheBahir

ApacheBahir Mar 28, 2017

Build failed, see build log for details

[INFO] --- scalatest-maven-plugin:1.0:test (test) @ spark-sql-streaming-akka_2.11 ---
...
*** RUN ABORTED ***
  java.lang.RuntimeException: Unable to load a Suite class that was discovered in the runpath: org.apache.bahir.sql.streaming.akka.AkkaStreamSourceSuite
  at org.scalatest.tools.DiscoverySuite$.getSuiteInstance(DiscoverySuite.scala:84)
  at org.scalatest.tools.DiscoverySuite$$anonfun$1.apply(DiscoverySuite.scala:38)
  at org.scalatest.tools.DiscoverySuite$$anonfun$1.apply(DiscoverySuite.scala:37)
  at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234)
  at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234)
  at scala.collection.Iterator$class.foreach(Iterator.scala:893)
  at scala.collection.AbstractIterator.foreach(Iterator.scala:1336)
  at scala.collection.IterableLike$class.foreach(IterableLike.scala:72)
  at scala.collection.AbstractIterable.foreach(Iterable.scala:54)
  at scala.collection.TraversableLike$class.map(TraversableLike.scala:234)
  ...
  Cause: org.jboss.netty.channel.ChannelException: Failed to bind to: /127.0.0.1:5150
  at org.jboss.netty.bootstrap.ServerBootstrap.bind(ServerBootstrap.java:272)
  at akka.remote.transport.netty.NettyTransport$$anonfun$listen$1.apply(NettyTransport.scala:393)
  at akka.remote.transport.netty.NettyTransport$$anonfun$listen$1.apply(NettyTransport.scala:389)
  at scala.util.Success$$anonfun$map$1.apply(Try.scala:237)
  at scala.util.Try$.apply(Try.scala:192)
  at scala.util.Success.map(Try.scala:237)
  at scala.concurrent.Future$$anonfun$map$1.apply(Future.scala:237)
  at scala.concurrent.Future$$anonfun$map$1.apply(Future.scala:237)
  at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:32)
  at akka.dispatch.BatchingExecutor$AbstractBatch.processBatch(BatchingExecutor.scala:55)
  ...
  Cause: java.net.BindException: Address already in use
  at sun.nio.ch.Net.bind0(Native Method)
  at sun.nio.ch.Net.bind(Net.java:433)
  at sun.nio.ch.Net.bind(Net.java:425)
  at sun.nio.ch.ServerSocketChannelImpl.bind(ServerSocketChannelImpl.java:223)
  at sun.nio.ch.ServerSocketAdaptor.bind(ServerSocketAdaptor.java:74)
  at org.jboss.netty.channel.socket.nio.NioServerBoss$RegisterTask.run(NioServerBoss.java:193)
  at org.jboss.netty.channel.socket.nio.AbstractNioSelector.processTaskQueue(AbstractNioSelector.java:372)
  at org.jboss.netty.channel.socket.nio.AbstractNioSelector.run(AbstractNioSelector.java:296)
  at org.jboss.netty.channel.socket.nio.NioServerBoss.run(NioServerBoss.java:42)
  at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
  ...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 22.872 s
[INFO] Finished at: 2017-03-27T23:39:00-07:00
[INFO] Final Memory: 44M/1030M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.scalatest:scalatest-maven-plugin:1.0:test (test) on project spark-sql-streaming-akka_2.11: There are test failures -> [Help 1]

ApacheBahir commented Mar 28, 2017

Build failed, see build log for details

[INFO] --- scalatest-maven-plugin:1.0:test (test) @ spark-sql-streaming-akka_2.11 ---
...
*** RUN ABORTED ***
  java.lang.RuntimeException: Unable to load a Suite class that was discovered in the runpath: org.apache.bahir.sql.streaming.akka.AkkaStreamSourceSuite
  at org.scalatest.tools.DiscoverySuite$.getSuiteInstance(DiscoverySuite.scala:84)
  at org.scalatest.tools.DiscoverySuite$$anonfun$1.apply(DiscoverySuite.scala:38)
  at org.scalatest.tools.DiscoverySuite$$anonfun$1.apply(DiscoverySuite.scala:37)
  at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234)
  at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234)
  at scala.collection.Iterator$class.foreach(Iterator.scala:893)
  at scala.collection.AbstractIterator.foreach(Iterator.scala:1336)
  at scala.collection.IterableLike$class.foreach(IterableLike.scala:72)
  at scala.collection.AbstractIterable.foreach(Iterable.scala:54)
  at scala.collection.TraversableLike$class.map(TraversableLike.scala:234)
  ...
  Cause: org.jboss.netty.channel.ChannelException: Failed to bind to: /127.0.0.1:5150
  at org.jboss.netty.bootstrap.ServerBootstrap.bind(ServerBootstrap.java:272)
  at akka.remote.transport.netty.NettyTransport$$anonfun$listen$1.apply(NettyTransport.scala:393)
  at akka.remote.transport.netty.NettyTransport$$anonfun$listen$1.apply(NettyTransport.scala:389)
  at scala.util.Success$$anonfun$map$1.apply(Try.scala:237)
  at scala.util.Try$.apply(Try.scala:192)
  at scala.util.Success.map(Try.scala:237)
  at scala.concurrent.Future$$anonfun$map$1.apply(Future.scala:237)
  at scala.concurrent.Future$$anonfun$map$1.apply(Future.scala:237)
  at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:32)
  at akka.dispatch.BatchingExecutor$AbstractBatch.processBatch(BatchingExecutor.scala:55)
  ...
  Cause: java.net.BindException: Address already in use
  at sun.nio.ch.Net.bind0(Native Method)
  at sun.nio.ch.Net.bind(Net.java:433)
  at sun.nio.ch.Net.bind(Net.java:425)
  at sun.nio.ch.ServerSocketChannelImpl.bind(ServerSocketChannelImpl.java:223)
  at sun.nio.ch.ServerSocketAdaptor.bind(ServerSocketAdaptor.java:74)
  at org.jboss.netty.channel.socket.nio.NioServerBoss$RegisterTask.run(NioServerBoss.java:193)
  at org.jboss.netty.channel.socket.nio.AbstractNioSelector.processTaskQueue(AbstractNioSelector.java:372)
  at org.jboss.netty.channel.socket.nio.AbstractNioSelector.run(AbstractNioSelector.java:296)
  at org.jboss.netty.channel.socket.nio.NioServerBoss.run(NioServerBoss.java:42)
  at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
  ...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 22.872 s
[INFO] Finished at: 2017-03-27T23:39:00-07:00
[INFO] Final Memory: 44M/1030M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.scalatest:scalatest-maven-plugin:1.0:test (test) on project spark-sql-streaming-akka_2.11: There are test failures -> [Help 1]
@ApacheBahir

This comment has been minimized.

Show comment
Hide comment
@ApacheBahir

ApacheBahir Mar 28, 2017

Refer to this link for build results (access rights to CI server needed):
http://169.45.79.58:8080/job/bahir_spark_pr_builder/37/console

ApacheBahir commented Mar 28, 2017

Refer to this link for build results (access rights to CI server needed):
http://169.45.79.58:8080/job/bahir_spark_pr_builder/37/console

@ckadner

This comment has been minimized.

Show comment
Hide comment
@ckadner

ckadner Mar 28, 2017

Member

@sbcd90 -- can you change your test suite to chose the akka.remote.netty.tcp.port dynamically?

Member

ckadner commented Mar 28, 2017

@sbcd90 -- can you change your test suite to chose the akka.remote.netty.tcp.port dynamically?

@ApacheBahir

This comment has been minimized.

Show comment
Hide comment
@ApacheBahir

ApacheBahir Mar 28, 2017

Build successful

ApacheBahir commented Mar 28, 2017

Build successful

@ApacheBahir

This comment has been minimized.

Show comment
Hide comment
@ApacheBahir

ApacheBahir Mar 28, 2017

Refer to this link for build results (access rights to CI server needed):
http://169.45.79.58:8080/job/bahir_spark_pr_builder/38/

ApacheBahir commented Mar 28, 2017

Refer to this link for build results (access rights to CI server needed):
http://169.45.79.58:8080/job/bahir_spark_pr_builder/38/

@sbcd90

This comment has been minimized.

Show comment
Hide comment
@sbcd90

sbcd90 Mar 28, 2017

Contributor

retest this please

Contributor

sbcd90 commented Mar 28, 2017

retest this please

@sbcd90

This comment has been minimized.

Show comment
Hide comment
@sbcd90

sbcd90 Mar 28, 2017

Contributor

Hi @ckadner , I have made akka.remote.netty.tcp.port dynamic.

Contributor

sbcd90 commented Mar 28, 2017

Hi @ckadner , I have made akka.remote.netty.tcp.port dynamic.

@sbcd90

This comment has been minimized.

Show comment
Hide comment
@sbcd90

sbcd90 Mar 30, 2017

Contributor

Hi @ckadner , the test cases are skipped..any reason for that?

Contributor

sbcd90 commented Mar 30, 2017

Hi @ckadner , the test cases are skipped..any reason for that?

@ckadner

This comment has been minimized.

Show comment
Hide comment
@ckadner

ckadner Mar 30, 2017

Member

@sbcd90 -- your test cases ran fine, just with a lot of "noise".

http://169.45.79.58:8080/job/bahir_spark_pr_builder/38/consoleFull

00:27:23 [INFO] --- scalatest-maven-plugin:1.0:test (test) @ spark-sql-streaming-akka_2.11 ---
...
00:27:24 Discovery starting.
00:27:24 log4j:ERROR Could not read configuration file from URL [file:src/test/resources/log4j.properties].
00:27:24 java.io.FileNotFoundException: src/test/resources/log4j.properties (No such file or directory)
...
00:27:24 log4j:ERROR Ignoring configuration file [file:src/test/resources/log4j.properties].
...
00:27:24 Discovery completed in 698 milliseconds.
00:27:24 Run starting. Expected test count is: 5
00:27:24 StressTestAkkaSource:
00:27:24 Using Spark's default log4j profile: org/apache/spark/log4j-defaults.properties
...
00:28:23 Run completed in 59 seconds, 534 milliseconds.
00:28:23 Total number of tests run: 5
00:28:23 Suites: completed 4, aborted 0
00:28:23 Tests: succeeded 5, failed 0, canceled 0, ignored 0, pending 0
00:28:23 All tests passed.

Could you add a log4.j.properties file in the test source folder to reduce the log verbosity?

Member

ckadner commented Mar 30, 2017

@sbcd90 -- your test cases ran fine, just with a lot of "noise".

http://169.45.79.58:8080/job/bahir_spark_pr_builder/38/consoleFull

00:27:23 [INFO] --- scalatest-maven-plugin:1.0:test (test) @ spark-sql-streaming-akka_2.11 ---
...
00:27:24 Discovery starting.
00:27:24 log4j:ERROR Could not read configuration file from URL [file:src/test/resources/log4j.properties].
00:27:24 java.io.FileNotFoundException: src/test/resources/log4j.properties (No such file or directory)
...
00:27:24 log4j:ERROR Ignoring configuration file [file:src/test/resources/log4j.properties].
...
00:27:24 Discovery completed in 698 milliseconds.
00:27:24 Run starting. Expected test count is: 5
00:27:24 StressTestAkkaSource:
00:27:24 Using Spark's default log4j profile: org/apache/spark/log4j-defaults.properties
...
00:28:23 Run completed in 59 seconds, 534 milliseconds.
00:28:23 Total number of tests run: 5
00:28:23 Suites: completed 4, aborted 0
00:28:23 Tests: succeeded 5, failed 0, canceled 0, ignored 0, pending 0
00:28:23 All tests passed.

Could you add a log4.j.properties file in the test source folder to reduce the log verbosity?

@sbcd90

This comment has been minimized.

Show comment
Hide comment
@sbcd90

sbcd90 Mar 30, 2017

Contributor

retest this please

Contributor

sbcd90 commented Mar 30, 2017

retest this please

@sbcd90

This comment has been minimized.

Show comment
Hide comment
@sbcd90

sbcd90 Mar 30, 2017

Contributor

Hi @ckadner , Thanks for your reply. I have added log4j.properties in the test sources folder.

Contributor

sbcd90 commented Mar 30, 2017

Hi @ckadner , Thanks for your reply. I have added log4j.properties in the test sources folder.

@ApacheBahir

This comment has been minimized.

Show comment
Hide comment
@ApacheBahir

ApacheBahir Mar 30, 2017

Build successful

ApacheBahir commented Mar 30, 2017

Build successful

@ApacheBahir

This comment has been minimized.

Show comment
Hide comment
@ApacheBahir

ApacheBahir Mar 30, 2017

Refer to this link for build results (access rights to CI server needed):
http://169.45.79.58:8080/job/bahir_spark_pr_builder/42/

ApacheBahir commented Mar 30, 2017

Refer to this link for build results (access rights to CI server needed):
http://169.45.79.58:8080/job/bahir_spark_pr_builder/42/

@ckadner

This comment has been minimized.

Show comment
Hide comment
@ckadner

ckadner Mar 31, 2017

Member

@sbcd90 -- Thanks, much better! To remove the remaining noise, I believe you would have to add a dependency to akka-slf4j in sql-streaming-akka/pom.xml and configure Akka to use the akka.event.slf4j.Slf4jLogger ...

    <dependency>
      <groupId>${akka.group}</groupId>
      <artifactId>akka-slf4j_${scala.binary.version}</artifactId>
      <version>${akka.version}</version>
    </dependency>
akka.loggers.0 = "akka.event.slf4j.Slf4jLogger"
akka.log-dead-letters-during-shutdown = "off"

Could you try that? This is something I had been meaning to do for our other Akka and Akka dependent tests.

Member

ckadner commented Mar 31, 2017

@sbcd90 -- Thanks, much better! To remove the remaining noise, I believe you would have to add a dependency to akka-slf4j in sql-streaming-akka/pom.xml and configure Akka to use the akka.event.slf4j.Slf4jLogger ...

    <dependency>
      <groupId>${akka.group}</groupId>
      <artifactId>akka-slf4j_${scala.binary.version}</artifactId>
      <version>${akka.version}</version>
    </dependency>
akka.loggers.0 = "akka.event.slf4j.Slf4jLogger"
akka.log-dead-letters-during-shutdown = "off"

Could you try that? This is something I had been meaning to do for our other Akka and Akka dependent tests.

@sbcd90

This comment has been minimized.

Show comment
Hide comment
@sbcd90

sbcd90 Mar 31, 2017

Contributor

retest this please

Contributor

sbcd90 commented Mar 31, 2017

retest this please

@sbcd90

This comment has been minimized.

Show comment
Hide comment
@sbcd90

sbcd90 Mar 31, 2017

Contributor

Hi @ckadner , I have made the changes you mentioned to remove the remaining verbose. Please have a look.

Contributor

sbcd90 commented Mar 31, 2017

Hi @ckadner , I have made the changes you mentioned to remove the remaining verbose. Please have a look.

@ApacheBahir

This comment has been minimized.

Show comment
Hide comment
@ApacheBahir

ApacheBahir Mar 31, 2017

Build successful

ApacheBahir commented Mar 31, 2017

Build successful

@ApacheBahir

This comment has been minimized.

Show comment
Hide comment
@ApacheBahir

ApacheBahir Mar 31, 2017

Refer to this link for build results (access rights to CI server needed):
http://169.45.79.58:8080/job/bahir_spark_pr_builder/48/

ApacheBahir commented Mar 31, 2017

Refer to this link for build results (access rights to CI server needed):
http://169.45.79.58:8080/job/bahir_spark_pr_builder/48/

@ckadner

minor not pick, please make sure to add new line characters at the end of files

</dependencySet>
</dependencySets>
</assembly>

This comment has been minimized.

@ckadner

ckadner Mar 31, 2017

Member

add new line

@ckadner

ckadner Mar 31, 2017

Member

add new line

</plugin>
</plugins>
</build>
</project>

This comment has been minimized.

@ckadner

ckadner Mar 31, 2017

Member

add new line

@ckadner

ckadner Mar 31, 2017

Member

add new line

}
loggers.0 = "akka.event.slf4j.Slf4jLogger"
log-dead-letters-during-shutdown = "off"
}

This comment has been minimized.

@ckadner

ckadner Mar 31, 2017

Member

add new line

@ckadner

ckadner Mar 31, 2017

Member

add new line

@ckadner

This comment has been minimized.

Show comment
Hide comment
@ckadner

ckadner Mar 31, 2017

Member

@sbcd90 -- thanks for you continuous updates and sorry for the piecemeal review from my end ... I started with your test cases since we were still in the process of fixing our Jenkins build setup. But your test cases are great now :-)

Perhaps more important, my first request to you should have been to add a README and examples so users can start using your connector without having to read through too much code ... i.e. please further follow the precedence set by sql-streaming-mqtt.

Thank you!

Member

ckadner commented Mar 31, 2017

@sbcd90 -- thanks for you continuous updates and sorry for the piecemeal review from my end ... I started with your test cases since we were still in the process of fixing our Jenkins build setup. But your test cases are great now :-)

Perhaps more important, my first request to you should have been to add a README and examples so users can start using your connector without having to read through too much code ... i.e. please further follow the precedence set by sql-streaming-mqtt.

Thank you!

@sbcd90

This comment has been minimized.

Show comment
Hide comment
@sbcd90

sbcd90 Apr 2, 2017

Contributor

retest this please

Contributor

sbcd90 commented Apr 2, 2017

retest this please

@ApacheBahir

This comment has been minimized.

Show comment
Hide comment
@ApacheBahir

ApacheBahir Apr 2, 2017

Build successful

ApacheBahir commented Apr 2, 2017

Build successful

@ApacheBahir

This comment has been minimized.

Show comment
Hide comment
@ApacheBahir

ApacheBahir Apr 2, 2017

Refer to this link for build results (access rights to CI server needed):
http://169.45.79.58:8080/job/bahir_spark_pr_builder/50/

ApacheBahir commented Apr 2, 2017

Refer to this link for build results (access rights to CI server needed):
http://169.45.79.58:8080/job/bahir_spark_pr_builder/50/

@sbcd90

This comment has been minimized.

Show comment
Hide comment
@sbcd90

sbcd90 Apr 2, 2017

Contributor

Hi @ckadner , I have provided a README & examples as well..
Also, I have added a newline at the end of all the files..
plz have a look..

Contributor

sbcd90 commented Apr 2, 2017

Hi @ckadner , I have provided a README & examples as well..
Also, I have added a newline at the end of all the files..
plz have a look..

@sbcd90

This comment has been minimized.

Show comment
Hide comment
@sbcd90

sbcd90 Apr 5, 2017

Contributor

Hi @ckadner , I haven't seen any updates from you..plz let me know if anything else is required from my side...

Contributor

sbcd90 commented Apr 5, 2017

Hi @ckadner , I haven't seen any updates from you..plz let me know if anything else is required from my side...

Show outdated Hide outdated ...st/scala/org/apache/bahir/sql/streaming/akka/AkkaStreamSourceSuite.scala
Show outdated Hide outdated ...st/scala/org/apache/bahir/sql/streaming/akka/AkkaStreamSourceSuite.scala
import java.nio.file.attribute.BasicFileAttributes
object BahirUtils extends Logging {

This comment has been minimized.

@lresende

lresende Apr 5, 2017

Member

We should not duplicate these two classes on each extension. Could you create a jira for refactoring these into a bahir-common project that is shared with the other extensions.

@lresende

lresende Apr 5, 2017

Member

We should not duplicate these two classes on each extension. Could you create a jira for refactoring these into a bahir-common project that is shared with the other extensions.

trait Logging {
final val log = LoggerFactory.getLogger(this.getClass.getName.stripSuffix("$"))
}

This comment has been minimized.

@lresende

lresende Apr 5, 2017

Member

Similar comment as file above.

@lresende

lresende Apr 5, 2017

Member

Similar comment as file above.

@lresende

This comment has been minimized.

Show comment
Hide comment
@lresende

lresende Apr 5, 2017

Member

Once we remove the deprecated api usages, we should be ready to merge unless someone has other comments.

Member

lresende commented Apr 5, 2017

Once we remove the deprecated api usages, we should be ready to merge unless someone has other comments.

@sbcd90

This comment has been minimized.

Show comment
Hide comment
@sbcd90

sbcd90 Apr 5, 2017

Contributor

retest this please

Contributor

sbcd90 commented Apr 5, 2017

retest this please

@ApacheBahir

This comment has been minimized.

Show comment
Hide comment
@ApacheBahir

ApacheBahir Apr 5, 2017

Build successful

ApacheBahir commented Apr 5, 2017

Build successful

@ApacheBahir

This comment has been minimized.

Show comment
Hide comment
@ApacheBahir

ApacheBahir Apr 5, 2017

Refer to this link for build results (access rights to CI server needed):
http://169.45.79.58:8080/job/bahir_spark_pr_builder/53/

ApacheBahir commented Apr 5, 2017

Refer to this link for build results (access rights to CI server needed):
http://169.45.79.58:8080/job/bahir_spark_pr_builder/53/

@sbcd90

This comment has been minimized.

Show comment
Hide comment
@sbcd90

sbcd90 Apr 5, 2017

Contributor

Hi @lresende , I have removed the usage of the deprecated api s
& also opened a jira issue named BAHIR-103
for refactoring the utils classes into bahir-common.

Contributor

sbcd90 commented Apr 5, 2017

Hi @lresende , I have removed the usage of the deprecated api s
& also opened a jira issue named BAHIR-103
for refactoring the utils classes into bahir-common.

@ckadner

This comment has been minimized.

Show comment
Hide comment
@ckadner

ckadner Apr 5, 2017

Member

Thanks @lresende and @sbcd90 - LGTM

Member

ckadner commented Apr 5, 2017

Thanks @lresende and @sbcd90 - LGTM

@asfgit asfgit closed this in 889de65 Apr 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment