[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-05-26 Thread Crystark
Github user Crystark commented on the pull request: https://github.com/apache/storm/pull/374#issuecomment-105472684 On more thing i noticed: [Column.hashCode](https://github.com/apache/storm/blob/master/external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/Column.java#L99)

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-05-26 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on the pull request: https://github.com/apache/storm/pull/374#issuecomment-105598508 @Crystark You are right I will fix it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-24 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/374#issuecomment-75847524 I am now getting a compilation error. ``` [ERROR] external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/Util.java:[30,29] incomparable types: int and

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-24 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/374#issuecomment-75848428 Yes it looks like it is a JDK8 issue. JDK6 compiles the code just fine. I'll file a new JIRA for it. --- If your project is set up for it, you can reply to this email

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-23 Thread harshach
Github user harshach commented on the pull request: https://github.com/apache/storm/pull/374#issuecomment-75650396 Thanks @Parth-Brahmbhatt merged into master. --- 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

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-23 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/374 --- 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

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-19 Thread Crystark
Github user Crystark commented on the pull request: https://github.com/apache/storm/pull/374#issuecomment-75028911 Works like a charm :) --- 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

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-19 Thread harshach
Github user harshach commented on the pull request: https://github.com/apache/storm/pull/374#issuecomment-75185107 +1. @Parth-Brahmbhatt Thanks for the quick fix . @Crystark Thanks for review and nice catch. @revans2 @ptgoetz I think this is ready to merge it in will give it

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-18 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on the pull request: https://github.com/apache/storm/pull/374#issuecomment-74940637 @Crystark super sorry about this I am not sure how the heck did I miss this. The code you pointed at is fine but I should be using executeBatch instead of

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-18 Thread Crystark
Github user Crystark commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24902452 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JdbcClient.java --- @@ -0,0 +1,213 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-18 Thread Crystark
Github user Crystark commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24902378 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JdbcClient.java --- @@ -0,0 +1,213 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-10 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24434272 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JdbcClient.java --- @@ -0,0 +1,213 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-10 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24461061 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcLookupBolt.java --- @@ -0,0 +1,84 @@ +/** + * Licensed to the

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24043926 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,81 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24044371 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/Column.java --- @@ -0,0 +1,87 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24015372 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/Column.java --- @@ -0,0 +1,87 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24014941 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,81 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/374#issuecomment-72685012 For the most part it looks OK. I am concerned about a few of the APIs seem confusing to me and could use some better javadocs. Also I am concerned with the performance

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24014635 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,81 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24014770 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,91 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24015210 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcLookupBolt.java --- @@ -0,0 +1,86 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24016216 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JdbcClient.java --- @@ -0,0 +1,213 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24037306 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,81 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24039560 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcLookupBolt.java --- @@ -0,0 +1,86 @@ +/** + * Licensed to the

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24039215 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,81 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24039240 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,81 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24037432 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,81 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24039341 --- Diff: external/storm-jdbc/pom.xml --- @@ -0,0 +1,125 @@ +?xml version=1.0 encoding=UTF-8? +!-- + Licensed to the Apache Software Foundation

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24040425 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JdbcClient.java --- @@ -0,0 +1,213 @@ +/** + * Licensed to the

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24040287 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/Column.java --- @@ -0,0 +1,87 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24041005 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JdbcClient.java --- @@ -0,0 +1,213 @@ +/** + * Licensed to the

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-26 Thread harshach
Github user harshach commented on the pull request: https://github.com/apache/storm/pull/374#issuecomment-71579088 @revans2 @ptgoetz can you take a look at 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.

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-21 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r23339923 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JDBCClient.java --- @@ -0,0 +1,213 @@ +/** + * Licensed to the

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-20 Thread itaifrenkel
Github user itaifrenkel commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r23282556 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JDBCClient.java --- @@ -0,0 +1,213 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22962128 --- Diff: external/storm-jdbc/README.md --- @@ -0,0 +1,208 @@ +#Storm JDBC +Storm/Trident integration for JDBC. This package includes the core

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread harshach
Github user harshach commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22962179 --- Diff: external/storm-jdbc/src/test/java/org/apache/storm/jdbc/topology/AbstractUserTopology.java --- @@ -0,0 +1,102 @@ +/** + * Licensed to the

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread itaifrenkel
Github user itaifrenkel commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22972354 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/AbstractJdbcBolt.java --- @@ -0,0 +1,53 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread harshach
Github user harshach commented on the pull request: https://github.com/apache/storm/pull/374#issuecomment-69978982 overall I am +1 on merging this. There are minor nit-picks in the above comments. I would like to see the unit tests for the bolt and state. Probably we can use

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22962079 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,91 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22962071 --- Diff: external/storm-jdbc/README.md --- @@ -0,0 +1,208 @@ +#Storm JDBC +Storm/Trident integration for JDBC. This package includes the core

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22962868 --- Diff: external/storm-jdbc/src/test/java/org/apache/storm/jdbc/topology/AbstractUserTopology.java --- @@ -0,0 +1,102 @@ +/** + * Licensed

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22974954 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,91 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22973996 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JDBCClient.java --- @@ -0,0 +1,209 @@ +/** + * Licensed to the

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22978604 --- Diff: external/storm-jdbc/README.md --- @@ -0,0 +1,208 @@ +#Storm JDBC +Storm/Trident integration for JDBC. This package includes the core

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22978468 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JDBCClient.java --- @@ -0,0 +1,209 @@ +/** + * Licensed to the

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread itaifrenkel
Github user itaifrenkel commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22973617 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcLookupBolt.java --- @@ -0,0 +1,80 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread itaifrenkel
Github user itaifrenkel commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22973557 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcLookupBolt.java --- @@ -0,0 +1,80 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread itaifrenkel
Github user itaifrenkel commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22973534 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,91 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread harshach
Github user harshach commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22974645 --- Diff: external/storm-jdbc/README.md --- @@ -0,0 +1,208 @@ +#Storm JDBC +Storm/Trident integration for JDBC. This package includes the core bolts

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread itaifrenkel
Github user itaifrenkel commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22973307 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,91 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread itaifrenkel
Github user itaifrenkel commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22972968 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JDBCClient.java --- @@ -0,0 +1,209 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread itaifrenkel
Github user itaifrenkel commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22975021 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JDBCClient.java --- @@ -0,0 +1,209 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22974973 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcLookupBolt.java --- @@ -0,0 +1,80 @@ +/** + * Licensed to the

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-14 Thread harshach
Github user harshach commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r22950826 --- Diff: external/storm-jdbc/README.md --- @@ -0,0 +1,208 @@ +#Storm JDBC +Storm/Trident integration for JDBC. This package includes the core bolts

[GitHub] storm pull request: Storm 616 : Storm-jdbc connector.

2015-01-06 Thread harshach
Github user harshach commented on the pull request: https://github.com/apache/storm/pull/372#issuecomment-68871971 @Parth-Brahmbhatt looks like you might have unintended commit as part of this PR. STORM-586: TridentKafkaEmitter should catch updateOffsetException. --- If your

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-01-06 Thread Parth-Brahmbhatt
GitHub user Parth-Brahmbhatt opened a pull request: https://github.com/apache/storm/pull/374 Storm-616 : Storm-jdbc connector. You can merge this pull request into a Git repository by running: $ git pull https://github.com/Parth-Brahmbhatt/incubator-storm STORM-616

[GitHub] storm pull request: Storm 616 : Storm-jdbc connector.

2015-01-06 Thread harshach
Github user harshach commented on a diff in the pull request: https://github.com/apache/storm/pull/372#discussion_r22536766 --- Diff: external/storm-jdbc/README.md --- @@ -0,0 +1,117 @@ +#Storm HBase + +Storm/Trident integration for JDBC. + +## Usage +The

[GitHub] storm pull request: Storm 616 : Storm-jdbc connector.

2015-01-06 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on the pull request: https://github.com/apache/storm/pull/372#issuecomment-68904233 * I noticed that and reverted those files to their state in master. * Added storm-jdbc to storm-core. * sorry about this, I accidently deleted it with

[GitHub] storm pull request: Storm 616 : Storm-jdbc connector.

2015-01-06 Thread harshach
Github user harshach commented on the pull request: https://github.com/apache/storm/pull/372#issuecomment-68899213 @Parth-Brahmbhatt few things to note 1) add external/storm-jdbc to storm/pom.xml under modules 2) I didn't' find any dependency in storm-jdbc/pom.xml for