Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/365
---
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 enabl
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-75576248
+1, sorry it took so long to get in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does n
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-74376629
Since we changed committer sponsor, seems like we can pull this in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on Git
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-73170761
@HeartSaVioR , @ptgoetz
I agree with "committer sponsors" only committers can join external's
"committer sponsors".
I have changed the committer sponsors membe
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-72779122
@dashengju
According to #416, we should remove ourselves from "committer sponsors" to
let storm-redis module can be merged into master.
Thanks for clarificatio
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-72309792
I wish to hear additional opinions, and finally see it pulled in master. :)
---
If your project is set up for it, you can reply to this email and have your
reply appea
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-71313044
I'll address follow-up issue, providing some persist / lookup bolts after
merged to master.
---
If your project is set up for it, you can reply to this email and have
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-71273044
+1 the changes still look good
Are there any other committers that want to take a look at the code? I
would feel more comfortable with more people looking at the
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-71028155
@dashengju
I don't think we ever explained it in the docs. I believe it was more of a
discussion on the mailing lists when the kafka integration code was brought
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-70978681
@dashengju
No, I mean that "What is Committer Sponsor and where can I find
description?".
I think if it is "Committer" Sponsor then we cannot be included, but
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-70974729
@HeartSaVioR , It is explained in README.md
---
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 proje
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-70936224
@revans2 @dashengju
Just I'm confused about 'Committer Sponsor'. Is it explained from docs? I
can't find it.
If it doesn't exist, could you explain it?
May
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-70777681
@revans2 , I have added you to the committer. 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
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-70755347
@Parth-Brahmbhatt
Thanks for review!
First of all, let's start about abstract bolt.
https://github.com/apache/storm/pull/365#issuecomment-68332013
Github user Parth-Brahmbhatt commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-70752983
@HeartSaVioR and @dashengju Thanks for the patch and I think overall the
code looks good.
I am little confused about the core bolt. You have a trident sta
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-70752747
@dashengju
For your information, Jedis team is discussing about Jedis 3.0, and
ShardedJedis could be replaced to JedisCluster for reducing code complexity.
htt
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/365#discussion_r23264315
--- Diff:
external/storm-redis/src/main/java/org/apache/storm/redis/trident/state/RedisClusterMapState.java
---
@@ -0,0 +1,295 @@
+/**
+ * License
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-70751193
@revans2
About tests, integration tests are based on storm-hdfs.
I'm not familiar with Storm unit tests (don't know @dashengju is familiar
with) so it takes so
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/365#discussion_r23263342
--- Diff:
external/storm-redis/src/main/java/org/apache/storm/redis/trident/state/RedisClusterMapState.java
---
@@ -0,0 +1,295 @@
+/**
+ * License
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/365#discussion_r23262904
--- Diff: external/storm-redis/pom.xml ---
@@ -0,0 +1,108 @@
+
+
+http://maven.apache.org/POM/4.0.0";
xmlns:xsi="http://www.w3.org/2001/XMLSche
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-70685389
My biggest comment is that there are some TODOs in the code that look like
they have already been addressed and should be deleted. I would like to see
some unit tests at
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/365#discussion_r23233009
--- Diff:
external/storm-redis/src/main/java/org/apache/storm/redis/trident/state/RedisClusterState.java
---
@@ -0,0 +1,81 @@
+/**
+ * Licensed to the
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/365#discussion_r23232733
--- Diff:
external/storm-redis/src/main/java/org/apache/storm/redis/trident/state/RedisClusterMapState.java
---
@@ -0,0 +1,295 @@
+/**
+ * Licensed to
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/365#discussion_r23232475
--- Diff: external/storm-redis/pom.xml ---
@@ -0,0 +1,108 @@
+
+
+http://maven.apache.org/POM/4.0.0";
xmlns:xsi="http://www.w3.org/2001/XMLSchema-i
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-70575248
@harshach @revans2 @ptgoetz
I'd like to hear your thoughts about this.
Anything including code reviews, thought about Storm + Redis, and so on is
very apprecia
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68803852
@dashengju Great! Thanks for applying.
---
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 user dashengju commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68692877
@HeartSaVioR , thanks for your careful review, I have changed all the notes.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user dashengju commented on a diff in the pull request:
https://github.com/apache/storm/pull/365#discussion_r22454671
--- Diff:
external/storm-redis/src/main/java/org/apache/storm/redis/trident/state/RedisClusterStateUpdater.java
---
@@ -0,0 +1,76 @@
+/**
+ * Licens
Github user dashengju commented on a diff in the pull request:
https://github.com/apache/storm/pull/365#discussion_r22454036
--- Diff:
external/storm-redis/src/main/java/org/apache/storm/redis/trident/state/RedisClusterStateQuerier.java
---
@@ -0,0 +1,81 @@
+/**
+ * Licens
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68652310
@dashengju
Though there're some points to reduce round-trip to Redis, looks good to
me. Great job!
Seems like we're ready to review our effort to Storm communi
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/365#discussion_r22443267
--- Diff:
external/storm-redis/src/main/java/org/apache/storm/redis/trident/state/RedisMapState.java
---
@@ -0,0 +1,327 @@
+/**
+ * Licensed to th
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/365#discussion_r22443235
--- Diff:
external/storm-redis/src/main/java/org/apache/storm/redis/trident/state/RedisClusterStateUpdater.java
---
@@ -0,0 +1,76 @@
+/**
+ * Lice
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/365#discussion_r22443134
--- Diff:
external/storm-redis/src/main/java/org/apache/storm/redis/trident/state/RedisClusterStateQuerier.java
---
@@ -0,0 +1,81 @@
+/**
+ * Lice
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68626207
@HeartSaVioR and I have added abstract bolt and trident state
implementations, we also add some test cases, and modify the readme usage.
Anyone can help review?
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68450496
@dashengju Thanks! Happy new year!
---
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 user dashengju commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68449366
okï¼I will add 2) 3) 4) those days.
@HeartSaVioR Happy New Year :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68448629
@dashengju
Supporting 2, 3, 4 seems OK. When we need 2 and/or 3, and/or 4 separately,
I don't think we need to maintain 1.
1 is just for unifying.
---
If you
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68447798
may be support 1) 2) 4) is ok
---
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 ha
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68447314
The users of our trident state, they may have a redis or have a redis
cluster. normally, they will not have both.
When they have a redis, they want to use the Jedis
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68437760
@dashengju
If we cannot tolerate single key operations restriction, there're some ways
to settle.
1. check type and convert, from usage of Jedis
We c
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68434697
@HeartSaVioR
If JedisCommands is designed for single key operation, It is useful for
RedisState, because we provide the JedisCommands interface to user and they can
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68431920
@dashengju
JedisCommands is for single key.
JedisCluster supports multi keys operation but all keys should be in one
slot (so it should be handled differently
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68431816
@harshach , we have added tests for both topology and trident.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68431637
@HeartSaVioR
I have added some test cases to trident, and I have ran success with all
the topology and trident test cases.
As the trident state, I think mge
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68428398
@HeartSaVioR , please help to review the JedisCommands usage in Trident
implementation.
---
If your project is set up for it, you can reply to this email and have your
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68426972
@dashengju Thanks for applying unification on Jedis / JedisCluster into
Trident implementation!
---
If your project is set up for it, you can reply to this email and
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68347429
@dashengju Thanks! And you can see we don't need to separate Jedis
implementation and JedisCluster implementation when we use JedisCommands. Hope
this helps. :D
---
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68346400
@HeartSaVioR I have merged your PR, thanks. And I have added Apache
licenses to all files.
I have found JedisCommands interface suitable for trident state, and
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68341375
@dashengju It would be great to move test codes to src/test/java.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHu
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68340267
I've implemented basic Redis bolt which supports JedisCommands (interface
for single key commands).
Please take a look if you're interested in.
https://git
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68334616
@darionyaphet
I also think providing Jedis instance could be enough for many users cause
Jedis is already easy to use.
So we can start from this case.
Github user darionyaphet commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68334201
Redis have a lot of function could do anything I have meet in my work .
It's hard to cover all pices of it and also needn't . You could just provide a
jedis instance
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68332013
Here're data types and commands I'm trying to support at first time.
Type | Read | Write
--- | --- | ---
STRING | GET (key) | SET (key, valu
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68330414
@darionyaphet
Hello.
For spout, it isn't acceptable for reliable data source because during
failure it'll lose some tuples.
(Actually Redis cannot act as r
Github user darionyaphet commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68327749
redis publish/subscribe is supported ?
---
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 user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68325672
@dashengju Thanks for concerning!
I'll share a sketch about what it looks like (maybe similar to storm-hbase)
and what types it can support.
---
If your project i
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68325551
@HeartSaVioR , ok , you can responsible for core storm bolt.
And I will add more data types for trident state and unit tests.
next time commit, I will add
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68324513
My sketch would be similar to storm-hbase, but because of various Redis
data types, supporting types needs design before implementing.
---
If your project is set up f
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68324419
@dashengju Oh, actually I'm sketching features for supporting core storm
bolt.
Could you wait for me to share sketches and let me implement it, or do you
want your
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68324052
@harshach , @Parth-Brahmbhatt , @HeartSaVioR
I have implemented a few functions in current PR.
As @HeartSaVioR says, it's better to collaborate with each other
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68322020
@dashengju Hey, it's better to collaborate with you by requesting PR to
merge.
I posted small PR to your fork just now.
---
If your project is set up for it, you
Github user Parth-Brahmbhatt commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68314612
Any reason to only provide implementation for trident? Do you plan to add
support for core storm bolt?
---
If your project is set up for it, you can reply to thi
Github user harshach commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68267817
@dashengju thanks for sharing this. I would like to see unit tests on the
code.
---
If your project is set up for it, you can reply to this email and have your
reply app
Github user dashengju commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68244243
@darionyaphet , I think storm-redis can support redis and redis-cluster
both. we can also add redis-sharding solution to trident state.
@HeartSaVioR , yes, I t
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68235532
If storm-redis is acceptable, maybe I can help improving module since I am
familiar with Redis, and I have been collaborating with Jedis. :D
---
If your project is se
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68235372
For example, you can't send mget/mset to multi keys which don't included to
same slot, which also means it should be in one machine.
---
If your project is set up for
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68235170
Since Redis Cluster has some limitations, it seems better to implement it
with Redis itself (and Jedis) unless state uses a big memory.
---
If your project is set up
Github user darionyaphet commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-68230852
Thank you for you great work . The storm-redis seems use redis cluster but
redis cluster is developing . Redis Sharding may be better ?
---
If your project is set u
GitHub user dashengju opened a pull request:
https://github.com/apache/storm/pull/365
[STORM-609] Add storm-redis to storm external
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dashengju/storm branch_storm_redis
Alternatively
69 matches
Mail list logo