[GitHub] storm pull request: STORM-851: Storm Solr Connector

2015-08-03 Thread hmcl
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/665 STORM-851: Storm Solr Connector 1. SolrUpdate Bolt 2. Trident State implementation 3. Fields Mapper 4. JSON Mapper 5. Integration Tests You can merge this pull request into a Git

[GitHub] storm pull request: STORM-851: Storm Solr Connector

2015-08-03 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/665#issuecomment-127483109 I am planning on pushing a few more unit tests while the community does the review. I have provided a set of functional tests that use the Solr gettingstarted example. I

[GitHub] storm pull request: STORM-851: Storm Solr Connector

2015-08-13 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/665#discussion_r36998880 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/schema/builder/RestJsonSchemaBuilder.java --- @@ -0,0 +1,53 @@ +package

[GitHub] storm pull request: STORM-851: Storm Solr Connector

2015-08-25 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/665#discussion_r37883322 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/bolt/SolrUpdateBolt.java --- @@ -0,0 +1,107 @@ +package org.apache.storm.solr.bolt

[GitHub] storm pull request: STORM-851: Storm Solr Connector

2015-08-25 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/665#discussion_r37883211 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/bolt/AbstractSolrBolt.java --- @@ -0,0 +1,33 @@ +package org.apache.storm.solr.bolt

[GitHub] storm pull request: STORM-851: Storm Solr Connector

2015-08-25 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/665#discussion_r37883364 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/bolt/AbstractSolrBolt.java --- @@ -0,0 +1,33 @@ +package org.apache.storm.solr.bolt

[GitHub] storm pull request: STORM-851: Storm Solr Connector

2015-08-25 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/665#discussion_r37883234 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/bolt/SolrUpdateBolt.java --- @@ -0,0 +1,107 @@ +package org.apache.storm.solr.bolt

[GitHub] storm pull request: STORM-851: Storm Solr Connector

2015-08-25 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/665#discussion_r37883137 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/mapper/SolrJsonMapper.java --- @@ -0,0 +1,97 @@ +package org.apache.storm.solr.mapper

[GitHub] storm pull request: STORM-851: Storm Solr Connector

2015-08-25 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/665#discussion_r37882952 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/mapper/SolrJsonMapper.java --- @@ -0,0 +1,97 @@ +package org.apache.storm.solr.mapper

[GitHub] storm pull request: STORM-1152 Change map keySet iteration to entr...

2015-10-30 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/836#discussion_r43549404 --- Diff: storm-core/src/jvm/storm/trident/topology/TridentTopologyBuilder.java --- @@ -233,8 +234,9 @@ public StormTopology buildTopology

[GitHub] storm pull request: STORM-1152 Change map keySet iteration to entr...

2015-10-30 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/836#discussion_r43548241 --- Diff: storm-core/src/jvm/backtype/storm/serialization/SerializationFactory.java --- @@ -81,8 +81,9 @@ public static Kryo getKryo(Map conf

[GitHub] storm pull request: STORM-1152 Change map keySet iteration to entr...

2015-10-30 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/836#issuecomment-152676783 +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

[GitHub] storm pull request: STORM-1152 Change map keySet iteration to entr...

2015-10-30 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/836#discussion_r43548584 --- Diff: storm-core/src/jvm/backtype/storm/utils/TransferDrainer.java --- @@ -30,18 +31,20 @@ private HashMap<Integer, ArrayList<ArrayList>

[GitHub] storm pull request: STORM-1152 Change map keySet iteration to entr...

2015-10-30 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/836#issuecomment-152646032 +1 LGTM. A few pinpoints only related with readability of the code that are left to the discretion of the implementer. --- If your project is set up for it, you can reply

[GitHub] storm pull request: STORM-1152 Change map keySet iteration to entr...

2015-10-30 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/836#discussion_r43549300 --- Diff: storm-core/src/jvm/storm/trident/spout/OpaquePartitionedTridentSpoutExecutor.java --- @@ -162,9 +162,9 @@ public void commit(TransactionAttempt

[GitHub] storm pull request: STORM-851: Storm Solr Connector

2015-08-25 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/665#discussion_r37928677 --- Diff: external/storm-solr/src/test/java/org/apache/storm/solr/topology/.gitignore --- @@ -0,0 +1 @@ +# Created by .ignore support plugin (hsz.mobi

[GitHub] storm pull request: STORM-851: Storm Solr Connector

2015-08-25 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/665#discussion_r37928671 --- Diff: pom.xml --- @@ -169,8 +169,9 @@ moduleexternal/storm-redis/module moduleexternal/storm-eventhubs/module

[GitHub] storm pull request: STORM-851: Storm Solr Connector

2015-08-25 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/665#discussion_r37928702 --- Diff: external/storm-solr/README.md --- @@ -0,0 +1,188 @@ +# Storm Solr +Storm and Trident integration for Apache Solr. This package includes a bolt

[GitHub] storm pull request: STORM-851: Storm Solr Connector

2015-08-25 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/665#discussion_r37928692 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/bolt/.gitignore --- @@ -0,0 +1 @@ +# Created by .ignore support plugin (hsz.mobi

[GitHub] storm pull request: STORM-851: Storm Solr Connector

2015-08-25 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/665#discussion_r37928682 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/mapper/.gitignore --- @@ -0,0 +1 @@ +# Created by .ignore support plugin (hsz.mobi

[GitHub] storm pull request: STORM-851: Storm Solr Connector

2015-08-25 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/665#discussion_r37928717 --- Diff: external/storm-solr/pom.xml --- @@ -0,0 +1,106 @@ +?xml version=1.0 encoding=UTF-8? +project xmlns=http://maven.apache.org/POM/4.0.0

[GitHub] storm pull request: STORM-1012: Shading jackson.

2015-09-15 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/702#issuecomment-140458502 +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

[GitHub] storm pull request: STORM-1179

2015-12-09 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/930#issuecomment-163363035 Currently, by default no Java or Clojure integration tests get run when executing `mvn clean install`. Only unit tests get executed. The user must run maven with one

[GitHub] storm pull request:

2015-12-09 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/commit/99f4494439ef4ab9c8a056ea5742221da08065c8#commitcomment-14882740 In documentation/storm-sql.md: In documentation/storm-sql.md on line 51: Shouldn't the Kafka properties here

[GitHub] storm pull request:

2015-12-09 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/commit/99f4494439ef4ab9c8a056ea5742221da08065c8#commitcomment-14881213 In documentation/storm-sql.md: In documentation/storm-sql.md on line 58: `ORDER` shouldn't it be `ORDERS` --- If your

[GitHub] storm pull request:

2015-12-09 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/commit/99f4494439ef4ab9c8a056ea5742221da08065c8#commitcomment-14881223 In documentation/storm-sql.md: In documentation/storm-sql.md on line 58: ORDER shouldn't it be ORDERS --- If your project

[GitHub] storm pull request:

2015-12-09 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/commit/99f4494439ef4ab9c8a056ea5742221da08065c8#commitcomment-14882118 In documentation/storm-sql.md: In documentation/storm-sql.md on line 59: ... clauses regardless of the table being read-only

[GitHub] storm pull request: STORM-1211 Added trident support for Cassandra...

2015-12-03 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/915#discussion_r46577241 --- Diff: external/storm-cassandra/src/main/java/org/apache/storm/cassandra/trident/state/CassandraState.java --- @@ -0,0 +1,149 @@ +/** + * Licensed

[GitHub] storm pull request: STORM-1211 Added trident support for Cassandra...

2015-12-03 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/915#discussion_r46577559 --- Diff: external/storm-cassandra/src/main/java/org/apache/storm/cassandra/trident/state/CassandraState.java --- @@ -0,0 +1,149 @@ +/** + * Licensed

[GitHub] storm pull request: STORM-1040. SQL support for Storm.

2015-12-03 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/914#issuecomment-161718691 +1. Nice job. --- 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

[GitHub] storm pull request: STORM-1211 Added trident support for Cassandra...

2015-12-03 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/915#discussion_r46578459 --- Diff: external/storm-cassandra/src/main/java/org/apache/storm/cassandra/trident/state/CassandraState.java --- @@ -0,0 +1,149 @@ +/** + * Licensed

[GitHub] storm pull request: STORM-1211 Added trident support for Cassandra...

2015-12-03 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/915#discussion_r46578817 --- Diff: external/storm-cassandra/src/test/java/org/apache/storm/cassandra/DynamicStatementBuilderTest.java --- @@ -116,15 +124,15 @@ private void

[GitHub] storm pull request: STORM-1211 Added trident support for Cassandra...

2015-12-03 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/915#discussion_r46579539 --- Diff: external/storm-cassandra/src/test/java/org/apache/storm/cassandra/trident/TridentTopologyTest.java --- @@ -0,0 +1,125 @@ +/** + * Licensed

[GitHub] storm pull request: STORM-1211 Added trident support for Cassandra...

2015-12-03 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/915#discussion_r46579509 --- Diff: external/storm-cassandra/src/test/java/org/apache/storm/cassandra/trident/TridentTopologyTest.java --- @@ -0,0 +1,125 @@ +/** + * Licensed

[GitHub] storm pull request: Storm 1179

2015-12-07 Thread hmcl
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/930 Storm 1179 This patch has 3 commits. - POM changes implementing profiles to separate integration tests and unit tests - Two separate commits that illustrate how to migrate existing tests

[GitHub] storm pull request: [STORM-1175] State store for windowing operati...

2015-12-10 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/939#discussion_r47261403 --- Diff: storm-core/test/jvm/backtype/storm/state/InMemoryKeyValueStateTest.java --- @@ -0,0 +1,73 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: STORM-1199 : HDFS Spout Functionally complete....

2015-12-16 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/936#discussion_r47847419 --- Diff: external/storm-hdfs/src/main/java/org/apache/storm/hdfs/common/CmpFilesByModificationTime.java --- @@ -0,0 +1,32 @@ +/** + * Licensed

[GitHub] storm pull request: STORM-1366. Add documentation for StormSQL int...

2016-01-05 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/931#discussion_r48886268 --- Diff: documentation/storm-sql-internal.md --- @@ -0,0 +1,55 @@ +--- +title: The Internals of Storm SQL +layout: documentation +documentation

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-07 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/977#discussion_r49138053 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/bolt/SolrUpdateBolt.java --- @@ -59,16 +64,24 @@ public SolrUpdateBolt(SolrConfig solrConfig

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-07 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/977#discussion_r49160816 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/bolt/SolrUpdateBolt.java --- @@ -92,9 +108,12 @@ private void ack(Tuple tuple) throws

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-07 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/977#discussion_r49160913 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/bolt/SolrUpdateBolt.java --- @@ -131,6 +150,21 @@ private void failQueuedTuples(List

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-07 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/977#discussion_r49160962 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/bolt/SolrUpdateBolt.java --- @@ -59,15 +63,24 @@ public SolrUpdateBolt(SolrConfig solrConfig

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-07 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/977#discussion_r49161066 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/config/SolrConfig.java --- @@ -39,4 +40,13 @@ public SolrConfig(String zkHostString

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-07 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/977#issuecomment-169908195 +1 once the last few, minor, comments are addressed. @vesense Thanks for the great work! --- If your project is set up for it, you can reply to this email and have

[GitHub] storm pull request: STORM-822 Implement Kafka 0.9 consumer API

2016-01-07 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/986#issuecomment-169910619 @Deepnekroz I am working on this. We wil make sure that all works well with what you have. --- If your project is set up for it, you can reply to this email and have your

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-09 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/977#discussion_r49267676 --- Diff: storm-core/src/jvm/backtype/storm/utils/TupleUtils.java --- @@ -28,8 +35,21 @@ private TupleUtils() { public static boolean isTick(Tuple

[GitHub] storm pull request: Storm SQL - Storm.py 'storm sql' startup scrip...

2016-01-09 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1001#issuecomment-170265013 @satishd @harshach Created the following JIRA to track creating unit tests for Storm.py https://issues.apache.org/jira/browse/STORM-1461 --- If your project is set up

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-09 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/977#discussion_r49266446 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/bolt/SolrUpdateBolt.java --- @@ -59,16 +63,29 @@ public SolrUpdateBolt(SolrConfig solrConfig

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-06 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/977#issuecomment-169436285 I would like to suggest that we address the tick tuples as follows: ![screen shot 2016-01-06 at 11 35 23 am](https://cloud.githubusercontent.com/assets/10284328

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-06 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/977#issuecomment-169438537 @HeartSaVioR All the tuples that have already been batched will still need to be acked, therefore we cannot simply ignore the tick tuple. All I am doing is that when

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-06 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/977#issuecomment-169441633 @revans2 your points are valid, as there may be scenarios where we may end up committing very small batches, which may cause some performance degradation, but in my opinion

[GitHub] storm pull request: [STORM-1420] solr CountBasedCommit should rese...

2016-01-07 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/978#issuecomment-169828477 @vesense why is this change 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

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-07 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/977#discussion_r49136481 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/bolt/SolrUpdateBolt.java --- @@ -92,9 +108,17 @@ private void ack(Tuple tuple) throws

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-07 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/977#discussion_r49136454 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/bolt/SolrUpdateBolt.java --- @@ -92,9 +108,17 @@ private void ack(Tuple tuple) throws

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-07 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/977#discussion_r49136662 --- Diff: external/storm-solr/src/main/java/org/apache/storm/solr/bolt/SolrUpdateBolt.java --- @@ -131,6 +155,26 @@ private void failQueuedTuples(List

[GitHub] storm pull request: STORM-1366. Add documentation for StormSQL int...

2016-01-08 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/931#discussion_r49259439 --- Diff: documentation/storm-sql.md --- @@ -0,0 +1,96 @@ +--- +title: Storm SQL integration +layout: documentation +documentation: true

[GitHub] storm pull request: Storm SQL - Storm.py 'storm sql' startup scrip...

2016-01-08 Thread hmcl
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/1001 Storm SQL - Storm.py 'storm sql' startup script is not working You can merge this pull request into a Git repository by running: $ git pull https://github.com/hmcl/storm-apache STORM_SQL_StormPY

[GitHub] storm pull request: STORM-1405: Add Maven 'all-tests' Profile to T...

2015-12-18 Thread hmcl
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/960 STORM-1405: Add Maven 'all-tests' Profile to Travis Script You can merge this pull request into a Git repository by running: $ git pull https://github.com/hmcl/storm-apache STORM-1405

[GitHub] storm pull request: STORM-1167: Add windowing support for storm co...

2015-11-20 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/855#discussion_r45482446 --- Diff: storm-core/src/jvm/backtype/storm/windowing/TupleWindow.java --- @@ -0,0 +1,26 @@ +/** + * Licensed to the Apache Software Foundation (ASF

[GitHub] storm pull request: STORM-1221. Create a common interface for all ...

2015-11-20 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/895#discussion_r45504842 --- Diff: storm-core/src/jvm/storm/trident/TridentTopology.java --- @@ -127,7 +128,21 @@ public Stream newStream(String txId, IPartitionedTridentSpout spout

[GitHub] storm pull request: STORM-1221. Create a common interface for all ...

2015-11-20 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/895#issuecomment-158495768 +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

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-05-27 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-78070 @jianbzhou I confirm that your suggested fix for doSeekRetriableTopicPartitions is correct. I am going to include that in the next patch. --- If your project is set up

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-05-27 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-78660 @connieyang I am finishing addressing some issues brought up by the initial users of this kafka spout, as well as unit test coverage, and will push the trident API right

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-05-27 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-80056 @jianbzhou any updates on ``` One customer of us who also use the spout they found some other issues: 1. Work load is not distributed to all spout tasks(as per

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-05-26 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-221928986 @jianbzhou looking at 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 project does not have

[GitHub] storm issue #1419: STORM-1838 update OffsetEntry to support EARLIEST/LATEST ...

2016-06-02 Thread hmcl
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1419 @flisky some stuff came up. Apologies for the delay in my response. I will get back to you today or tomorrow as I am working on this now. --- If your project is set up for it, you can reply

[GitHub] storm pull request #1451: STORM-1136: Command line module to return kafka sp...

2016-06-02 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/1451#discussion_r65576093 --- Diff: external/storm-kafka-monitor/src/main/java/org/apache/storm/kafka/monitor/KafkaOffsetLagUtil.java --- @@ -0,0 +1,374 @@ +/* + * Licensed

[GitHub] storm pull request #1451: STORM-1136: Command line module to return kafka sp...

2016-06-02 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/1451#discussion_r65576345 --- Diff: external/storm-kafka-monitor/src/main/java/org/apache/storm/kafka/monitor/KafkaOffsetLagUtil.java --- @@ -0,0 +1,374 @@ +/* + * Licensed

[GitHub] storm issue #1131: STORM-822: Kafka Spout New Consumer API

2016-06-02 Thread hmcl
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1131 @jianbzhou thanks. Looking at 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 project does not have this feature enabled

[GitHub] storm issue #1352: STORM-1723 (1.x) Introduce ClusterMetricsConsumer

2016-06-02 Thread hmcl
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1352 +1. I am also in favor of making cluster summary modular. --- 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

[GitHub] storm pull request: STORM-1861 Fix storm script bug to not fork ja...

2016-05-26 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1448#issuecomment-222035081 +1. @csivaguru can you please squash the commits. It will keep the log a bit cleaner --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-05-31 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131 @jianbzhou **Storm** guarantees that all the messages are either acked or failed. There is the property "topology.message.timeout.secs" https://github.com/apache/storm/blob/master/stor

[GitHub] storm issue #1487: off-by-one error for UNCOMMITTED_LATEST and UNCOMMITTED_E...

2016-06-16 Thread hmcl
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1487 @nabhanelrahman I am addressing this and other edge cases in a patch that I am about to submit. I am afraid that just removing the +1 won't work for all the cases. --- If your project is set up

[GitHub] storm pull request #1491: STORM-1907: PartitionedTridentSpoutExecutor has in...

2016-06-15 Thread hmcl
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/1491 STORM-1907: PartitionedTridentSpoutExecutor has incompatible types that cause ClassCastException - Generify on Object rather than Integer because example is broken - Fix TridentWordCount example

[GitHub] storm issue #1491: STORM-1907: PartitionedTridentSpoutExecutor has incompati...

2016-06-15 Thread hmcl
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1491 This PR is related to https://github.com/apache/storm/pull/1429 but I found this issue independently, in my local branch, which was a bit outdated. This patch still makes sense since it gets rid

[GitHub] storm issue #1431: STORM-1849: HDFSFileTopology should use the third argumen...

2016-06-16 Thread hmcl
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1431 +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

[GitHub] storm issue #1474: support for wildcard topics ( storm-kafka-client )

2016-06-23 Thread hmcl
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1474 @nabhanelrahman your patch enlightened that it would be better to add the `emit` and `getOutputFields` methods to `KafkaSpoutStream`. Furthermore, there were a couple of design decisions that would

[GitHub] storm issue #1516: STORM-1930: Kafka New Client API - Support for Topic Wild...

2016-06-23 Thread hmcl
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1516 @harshach Should this patch be merged onto 1.x-branch as well. 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

[GitHub] storm pull request #1516: STORM-1930: Kafka New Client API - Support for Top...

2016-06-23 Thread hmcl
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/1516 STORM-1930: Kafka New Client API - Support for Topic Wildcards - Interfaces for TuplesBuilder and KafkaSpoutStreams and implementations for named topics and wildcards - Test topologies for named

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-10 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/977#issuecomment-170373919 +1 LGTM @vesense to solve the conflicts you can rebase --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] storm pull request: Storm SQL - Storm.py 'storm sql' startup scrip...

2016-01-11 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1001#issuecomment-170661183 @revans2 without this fix the command line option 'storm sql' is broken. The reason it is broken is because prior to this change the method parameter 'topo_nam

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-13 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/977#issuecomment-171332402 +1 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

[GitHub] storm pull request: STORM-822 Implement Kafka 0.9 consumer API

2016-02-08 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/986#issuecomment-181721528 Hi @connieyang @jianbzhou apologies for not replying to you earlier. I was sidetracked with a last minute release requirement that wasn't planned and that got things a bit

[GitHub] storm pull request: STORM-1566 Passing File instance instead of pa...

2016-02-22 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1133#issuecomment-187365557 +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

[GitHub] storm pull request: Fix Log4j2.xml config to output the the timest...

2016-02-23 Thread hmcl
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/1145 Fix Log4j2.xml config to output the the timestamp in HH:mm:ss.SSS Currently the -r option outputs the number of milliseconds elapsed from the construction of the layout until the creation

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-02-26 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-189457987 @tgravescs @revans2 I am just finalizing some testing and I will push in the patch after lunch. --- If your project is set up for it, you can reply to this email

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-02-28 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-189923836 @tgravescs @revans2 I have pushed the latest changes. Please let me know of any feedback or further requirements you may have. Thanks. --- If your project is set up

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-02-25 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/1131#discussion_r54144192 --- Diff: external/storm-kafka-new-consumer-api/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java --- @@ -0,0 +1,457 @@ +/* + * Licensed

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-02-25 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-188995178 @revans2 no worries. Concerning your code snippet suggestion, I agree with most of it. We can definitely keep a lot of the state in the MessageId object. I agree

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-02-22 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/1131#discussion_r53723835 --- Diff: external/storm-kafka-new-consumer-api/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java --- @@ -0,0 +1,457 @@ +/* + * Licensed

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-02-22 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/1131#discussion_r53723733 --- Diff: external/storm-kafka-new-consumer-api/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java --- @@ -0,0 +1,457 @@ +/* + * Licensed

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-02-22 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/1131#discussion_r53723864 --- Diff: external/storm-kafka-new-consumer-api/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java --- @@ -0,0 +1,457 @@ +/* + * Licensed

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-02-22 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/1131#discussion_r53724182 --- Diff: external/storm-kafka-new-consumer-api/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java --- @@ -0,0 +1,457 @@ +/* + * Licensed

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-02-22 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/1131#discussion_r53724264 --- Diff: external/storm-kafka-new-consumer-api/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java --- @@ -0,0 +1,457 @@ +/* + * Licensed

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-02-22 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-187284409 @tgravescs trident will be appended to this patch. In the meantime it would be helpful if you could let me know of any other requirements that you may have

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-02-22 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/1131#discussion_r53659580 --- Diff: external/storm-kafka-new-consumer-api/src/main/java/org/apache/storm/kafka/spout/KafkaRecordTupleBuilder.java --- @@ -0,0 +1,34

[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API

2016-02-25 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-188859042 @tgravescs probably there is a bit of miscommunication here. I didn't complete the spout part yet because I have been waiting on getting answers to some follow up

[GitHub] storm pull request: STORM-822 Implement Kafka 0.9 consumer API

2016-01-19 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/986#issuecomment-172962722 @tgravescs I am actively working on this and I am going to conclude my implementation regardless of the status of the proposed patches. This task was and still is my

[GitHub] storm pull request: STORM-822 Implement Kafka 0.9 consumer API

2016-01-21 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/986#issuecomment-173498063 @connieyang the goal of my patch is to entirely replace the existing kafka spout; that includes the Trident API. In order to allow for a smooth migration, as well as avoid

[GitHub] storm pull request: STORM-822 Implement Kafka 0.9 consumer API

2016-01-25 Thread hmcl
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/986#issuecomment-174599255 @connieyang I will try go have a patch for review by the end of this week. I will also try to keep you posted on the progress. Thanks! --- If your project is set up

  1   2   3   4   5   6   7   8   >