You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by kamleshbhatt <gi...@git.apache.org> on 2017/02/21 07:53:45 UTC
[GitHub] storm pull request #1949: STORM-1290: port backtype.storm.local-state-test t...
GitHub user kamleshbhatt opened a pull request:
https://github.com/apache/storm/pull/1949
STORM-1290: port backtype.storm.local-state-test to java
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kamleshbhatt/storm master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/1949.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1949
----
commit da1c92c1f1a3c659532ce03d11a3ea376aa16e6c
Author: kamleshbhatt <kb...@gmail.com>
Date: 2016-12-05T18:52:33Z
STORM-1308: tick-tuple-test conversion to Java
commit eb2ca1cc12ad44525fba8025b0f6f19b19cc4a51
Author: kamleshbhatt <kb...@gmail.com>
Date: 2016-12-11T06:47:34Z
Merge branch 'master' of https://github.com/apache/storm
commit 50bebd90eabcce0bdaf4d5cb7d65e92bbfbe4985
Author: kamleshbhatt <kb...@gmail.com>
Date: 2016-12-11T06:51:07Z
fix
commit ffcefd5c5e020a7d149b3fe5bc3d3b1173d35552
Author: kamleshbhatt <kb...@gmail.com>
Date: 2016-12-11T06:56:35Z
remove unwanted file
commit ffc00dc44b1180312df750351778385e236cdffc
Author: kamleshbhatt <kb...@gmail.com>
Date: 2016-12-11T06:58:58Z
removed old clj test
commit d95769a65e482246258bba1736366e55260db866
Author: kamleshbhatt <kb...@gmail.com>
Date: 2017-01-17T17:39:58Z
Merge branch 'master' of https://github.com/kamleshbhatt/storm
commit 4a0ccf7f8bf12acf685b095d101b550d7901efcb
Author: kamleshbhatt <kb...@gmail.com>
Date: 2017-01-17T18:05:26Z
STORM-1292: port backtype.storm.messaging-test to java
commit db06090ad22b2573a633faf235b4447fc9e4204d
Author: kamleshbhatt <kb...@gmail.com>
Date: 2017-01-17T18:08:08Z
changes done
commit a8e07e7c0e0c0e33b1f4c8303b245b5af6bc81f2
Author: kamleshbhatt <kb...@gmail.com>
Date: 2017-01-17T18:13:00Z
STORM-1292: port backtype.storm.messaging-test to java
commit 2dc87c477b36937d4e921631961aadbb2b0b031d
Author: kamleshbhatt <kb...@gmail.com>
Date: 2017-01-17T18:05:26Z
STORM-1292: port backtype.storm.messaging-test to java
changes done
STORM-1292: port backtype.storm.messaging-test to java
commit cf68e95a53f1d8670385cf7b129f62154c5fca73
Author: kamleshbhatt <kb...@gmail.com>
Date: 2017-01-19T17:34:39Z
Merge remote-tracking branch 'origin/master'
commit 78403790e5f6b8c3e5ddc7e9f393e36947604273
Author: kamleshbhatt <kb...@gmail.com>
Date: 2017-01-19T17:37:30Z
STORM-1292: port backtype.storm.messaging-test to java
commit 5253704e28a5e5cc7f58a13af308558bed110aa0
Author: kamleshbhatt <kb...@gmail.com>
Date: 2017-02-21T07:11:18Z
update
commit 86859df609ef628b53bdab4b18e9643e2e9e7951
Author: kamleshbhatt <kb...@gmail.com>
Date: 2017-02-21T07:17:47Z
update
commit c6c3fa0b5e8967be8555a4f70a345b7a364c8c5b
Author: kamleshbhatt <kb...@gmail.com>
Date: 2017-02-21T07:50:09Z
local state test port to java
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1949: STORM-1290: port backtype.storm.local-state-test to java
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1949
@kamleshbhatt Any updates?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1949: STORM-1290: port backtype.storm.local-state-test to java
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1949
Could you create a new branch based on current master and work from there, and submit PR?
You need to pull master before starting.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #1949: STORM-1290: port backtype.storm.local-state-test t...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/1949#discussion_r104635268
--- Diff: storm-core/test/jvm/org/apache/storm/LocalStateTest.java ---
@@ -0,0 +1,54 @@
+package org.apache.storm;
+
+import org.apache.storm.generated.GlobalStreamId;
+import org.apache.storm.testing.TmpPath;
+import org.apache.storm.utils.LocalState;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.IOException;
+
+public class LocalStateTest {
+
+ @Test
+ public void testLocalState() throws IOException{
+ TmpPath dir1_tmp = new TmpPath();
+ TmpPath dir2_tmp = new TmpPath();
+ GlobalStreamId globalStreamId_a = new GlobalStreamId("a","a");
+ GlobalStreamId globalStreamId_b = new GlobalStreamId("b","b");
+ GlobalStreamId globalStreamId_c = new GlobalStreamId("c","c");
+ GlobalStreamId globalStreamId_d = new GlobalStreamId("d","d");
+
+ LocalState ls1 = new LocalState(dir1_tmp.getPath());
+ LocalState ls2 = new LocalState(dir2_tmp.getPath());
+ Assert.assertTrue(ls1.snapshot().isEmpty());
+
+ ls1.put("a",globalStreamId_a);
+ ls1.put("b",globalStreamId_b);
+ Assert.assertTrue(ls1.snapshot().containsKey("a"));
+ Assert.assertTrue(ls1.snapshot().containsValue(globalStreamId_a));
+ Assert.assertTrue(ls1.snapshot().containsKey("b"));
+ Assert.assertTrue(ls1.snapshot().containsValue(globalStreamId_b));
+
--- End diff --
You seemed to miss below lines:
```
- (is (= {} (.snapshot ls2)))
- (is (= gs-a (.get ls1 "a")))
- (is (= nil (.get ls1 "c")))
- (is (= gs-b (.get ls1 "b")))
- (is (= {"a" gs-a "b" gs-b} (.snapshot (LocalState. dir1))))
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #1949: STORM-1290: port backtype.storm.local-state-test t...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/1949#discussion_r104633614
--- Diff: storm-core/test/jvm/org/apache/storm/LocalStateTest.java ---
@@ -0,0 +1,54 @@
+package org.apache.storm;
+
+import org.apache.storm.generated.GlobalStreamId;
+import org.apache.storm.testing.TmpPath;
+import org.apache.storm.utils.LocalState;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.IOException;
+
+public class LocalStateTest {
+
+ @Test
+ public void testLocalState() throws IOException{
+ TmpPath dir1_tmp = new TmpPath();
--- End diff --
Let's use try-with-resource to make it compatible with `with-open` in Clojure. It should cover dir1_tmp and dir2_tmp.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1949: STORM-1290: port backtype.storm.local-state-test to java
Posted by kamleshbhatt <gi...@git.apache.org>.
Github user kamleshbhatt commented on the issue:
https://github.com/apache/storm/pull/1949
My sincere apology for the delay on this. As you suggested I am closing this PR and submit the revised one soon.
Thanks for all your help.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #1949: STORM-1290: port backtype.storm.local-state-test t...
Posted by kamleshbhatt <gi...@git.apache.org>.
Github user kamleshbhatt commented on a diff in the pull request:
https://github.com/apache/storm/pull/1949#discussion_r102148450
--- Diff: storm-core/test/jvm/org/apache/storm/LocalStateTest.java ---
@@ -0,0 +1,54 @@
+package org.apache.storm;
+
+import org.apache.storm.generated.GlobalStreamId;
+import org.apache.storm.testing.TmpPath;
+import org.apache.storm.utils.LocalState;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.IOException;
+
+public class LocalStateTest {
+
+ @Test
+ public void testLocalState() throws IOException{
+ TmpPath dir1_tmp = new TmpPath();
+ TmpPath dir2_tmp = new TmpPath();
+ GlobalStreamId globalStreamId_a = new GlobalStreamId("a","a");
+ GlobalStreamId globalStreamId_b = new GlobalStreamId("b","b");
+ GlobalStreamId globalStreamId_c = new GlobalStreamId("c","c");
+ GlobalStreamId globalStreamId_d = new GlobalStreamId("d","d");
+
+ LocalState ls1 = new LocalState(dir1_tmp.getPath());
+ LocalState ls2 = new LocalState(dir2_tmp.getPath());
+ Assert.assertTrue(ls1.snapshot().isEmpty());
+
+ ls1.put("a",globalStreamId_a);
+ ls1.put("b",globalStreamId_b);
+ Assert.assertTrue(ls1.snapshot().containsKey("a"));
+ Assert.assertTrue(ls1.snapshot().containsValue(globalStreamId_a));
+ Assert.assertTrue(ls1.snapshot().containsKey("b"));
+ Assert.assertTrue(ls1.snapshot().containsValue(globalStreamId_b));
+
+ ls2.put("b",globalStreamId_a);
+ ls2.put("b",globalStreamId_b);
+ ls2.put("b",globalStreamId_c);
+ ls2.put("b",globalStreamId_d);
+ Assert.assertEquals(globalStreamId_d, ls2.get("b"));
+ }
+
--- End diff --
I think testEmptyState is redundant. All these features are already tested in above testLocalState.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #1949: STORM-1290: port backtype.storm.local-state-test t...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/1949#discussion_r104634731
--- Diff: storm-core/test/jvm/org/apache/storm/LocalStateTest.java ---
@@ -0,0 +1,54 @@
+package org.apache.storm;
+
+import org.apache.storm.generated.GlobalStreamId;
+import org.apache.storm.testing.TmpPath;
+import org.apache.storm.utils.LocalState;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.IOException;
+
+public class LocalStateTest {
+
+ @Test
+ public void testLocalState() throws IOException{
+ TmpPath dir1_tmp = new TmpPath();
+ TmpPath dir2_tmp = new TmpPath();
+ GlobalStreamId globalStreamId_a = new GlobalStreamId("a","a");
+ GlobalStreamId globalStreamId_b = new GlobalStreamId("b","b");
+ GlobalStreamId globalStreamId_c = new GlobalStreamId("c","c");
+ GlobalStreamId globalStreamId_d = new GlobalStreamId("d","d");
+
+ LocalState ls1 = new LocalState(dir1_tmp.getPath());
+ LocalState ls2 = new LocalState(dir2_tmp.getPath());
+ Assert.assertTrue(ls1.snapshot().isEmpty());
+
+ ls1.put("a",globalStreamId_a);
+ ls1.put("b",globalStreamId_b);
+ Assert.assertTrue(ls1.snapshot().containsKey("a"));
--- End diff --
Below comparisons are broken: `{"a" globalStreamId_b "b" globalStreamId_a}` will pass.
I guess you can compare two Maps directly since Map.equals defines how implementation should compare each other properly.
https://docs.oracle.com/javase/7/docs/api/java/util/Map.html#equals(java.lang.Object)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #1949: STORM-1290: port backtype.storm.local-state-test t...
Posted by kamleshbhatt <gi...@git.apache.org>.
Github user kamleshbhatt closed the pull request at:
https://github.com/apache/storm/pull/1949
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1949: STORM-1290: port backtype.storm.local-state-test to java
Posted by kamleshbhatt <gi...@git.apache.org>.
Github user kamleshbhatt commented on the issue:
https://github.com/apache/storm/pull/1949
@HeartSaVioR ,my apology for not responding to this early as some urgent travel came up for me. Thanks for reviewing the PR. As you suggested I'll apply the review comments and submit a separate PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #1949: STORM-1290: port backtype.storm.local-state-test t...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/1949#discussion_r104635865
--- Diff: storm-core/test/jvm/org/apache/storm/LocalStateTest.java ---
@@ -0,0 +1,54 @@
+package org.apache.storm;
+
+import org.apache.storm.generated.GlobalStreamId;
+import org.apache.storm.testing.TmpPath;
+import org.apache.storm.utils.LocalState;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.IOException;
+
+public class LocalStateTest {
+
+ @Test
+ public void testLocalState() throws IOException{
+ TmpPath dir1_tmp = new TmpPath();
+ TmpPath dir2_tmp = new TmpPath();
+ GlobalStreamId globalStreamId_a = new GlobalStreamId("a","a");
+ GlobalStreamId globalStreamId_b = new GlobalStreamId("b","b");
+ GlobalStreamId globalStreamId_c = new GlobalStreamId("c","c");
+ GlobalStreamId globalStreamId_d = new GlobalStreamId("d","d");
+
+ LocalState ls1 = new LocalState(dir1_tmp.getPath());
+ LocalState ls2 = new LocalState(dir2_tmp.getPath());
+ Assert.assertTrue(ls1.snapshot().isEmpty());
+
+ ls1.put("a",globalStreamId_a);
+ ls1.put("b",globalStreamId_b);
+ Assert.assertTrue(ls1.snapshot().containsKey("a"));
+ Assert.assertTrue(ls1.snapshot().containsValue(globalStreamId_a));
+ Assert.assertTrue(ls1.snapshot().containsKey("b"));
+ Assert.assertTrue(ls1.snapshot().containsValue(globalStreamId_b));
+
+ ls2.put("b",globalStreamId_a);
+ ls2.put("b",globalStreamId_b);
+ ls2.put("b",globalStreamId_c);
+ ls2.put("b",globalStreamId_d);
+ Assert.assertEquals(globalStreamId_d, ls2.get("b"));
+ }
+
--- End diff --
I also guess so. You can remove 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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---