Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2443
---
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2443
@srdo Thanks for the detailed review and nice suggestions!
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2443
I think all my comments were addressed, thanks for your patience. LGTM, +1.
---
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2711
@roshannaik
https://travis-ci.org/apache/storm/jobs/403972342
https://travis-ci.org/apache/storm/jobs/403972347
https://travis-ci.org/apache/storm/jobs/403972348
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2443
@srdo I'd like to get final review and approval from you to check if I miss
anything before merging. Could you please help to do this? Thanks!
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2443
Looks great.
---
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2443
@srdo Nice finding. Fixed. Also exported to xml once again. Please check
again. Thanks for your patience!
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2443
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2443
@srdo
I have exported these diagrams into xml : please download and rename below
files to remove `.txt` (github doesn't allow uploading xml file directly) and
see they're properly imported
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2443
Raised https://issues.apache.org/jira/browse/STORM-3153 for addressing
restoring tests.
---
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202684941
--- Diff: docs/storm-sql-internal.md ---
@@ -1,59 +0,0 @@
--- End diff --
Nice finding. Will fix it and also will attach source
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202683106
--- Diff: docs/storm-sql-internal.md ---
@@ -1,59 +0,0 @@
--- End diff --
The second diagram on the page also mentions Trident. I don't
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202682665
--- Diff: docs/storm-sql-internal.md ---
@@ -1,59 +0,0 @@
--- End diff --
Thanks, looks great. I think the dotted line box is missing the
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202678116
--- Diff: docs/storm-sql-internal.md ---
@@ -1,59 +0,0 @@
--- End diff --
Just finished it.
---
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202674112
--- Diff: docs/storm-sql-internal.md ---
@@ -1,59 +0,0 @@
--- End diff --
Yeah right. I didn't think about that. Drawing a new one.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2443
Sounds good.
---
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2443
Yes test failures look unrelated, one is from server, another one is from
cassandra.
I'd like to address
https://github.com/apache/storm/pull/2443#discussion_r202639091 but OK to
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202653817
--- Diff: docs/storm-sql-internal.md ---
@@ -1,59 +0,0 @@
--- End diff --
It would probably be pretty quick to duplicate with e.g.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2443
The test failures look unrelated, not sure why it's suddenly failing. If
they don't go away by rebasing to master, we might want to raise issues to
track them.
I don't know if you're looking
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2443
I don't think we need to change it. I just wanted to make a note of it.
When this has been merged, we might want to add a couple of notes about
debugging to the docs, we can mention the log line there.
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202650814
--- Diff:
sql/storm-sql-runtime/src/jvm/org/apache/storm/sql/runtime/datasource/socket/bolt/SocketBolt.java
---
@@ -0,0 +1,108 @@
+/*
+ * Licensed to
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202650588
--- Diff:
sql/storm-sql-external/storm-sql-mongodb/src/jvm/org/apache/storm/sql/mongodb/MongoDataSourcesProvider.java
---
@@ -47,54 +45,60 @@
* The
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202650461
--- Diff:
sql/storm-sql-core/src/jvm/org/apache/storm/sql/planner/streams/StreamsStormRuleSets.java
---
@@ -72,13 +72,13 @@
// merge and push
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202644352
--- Diff: docs/storm-sql-internal.md ---
@@ -1,59 +0,0 @@
--- End diff --
We could even do some photo editing but not sure it is
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202639091
--- Diff:
sql/storm-sql-external/storm-sql-hdfs/src/test/org/apache/storm/sql/hdfs/TestHdfsDataSourcesProvider.java
---
@@ -88,46 +75,12 @@ public void
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202635190
--- Diff:
sql/storm-sql-external/storm-sql-mongodb/src/jvm/org/apache/storm/sql/mongodb/MongoDataSourcesProvider.java
---
@@ -47,54 +45,60 @@
*
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202631700
--- Diff: sql/README.md ---
@@ -1,187 +1,8 @@
# Storm SQL
-Compile SQL queries to Storm topologies.
+Compile SQL queries to Storm
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202632910
--- Diff:
sql/storm-sql-core/src/jvm/org/apache/storm/sql/planner/streams/StreamsStormRuleSets.java
---
@@ -72,13 +72,13 @@
// merge
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202638476
--- Diff:
sql/storm-sql-runtime/src/test/org/apache/storm/sql/TestUtils.java ---
@@ -41,7 +26,46 @@
import java.util.Map;
import
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202638239
--- Diff:
sql/storm-sql-runtime/src/jvm/org/apache/storm/sql/runtime/datasource/socket/bolt/SocketBolt.java
---
@@ -0,0 +1,108 @@
+/*
+ *
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202633872
--- Diff:
sql/storm-sql-core/src/jvm/org/apache/storm/sql/planner/streams/rel/StreamsStreamInsertRel.java
---
@@ -0,0 +1,82 @@
+/*
+ * Licensed
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202631939
--- Diff:
sql/storm-sql-core/src/jvm/org/apache/storm/sql/compiler/CompilerUtil.java ---
@@ -118,16 +138,32 @@ public RelDataType getRowType(
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202638350
--- Diff:
sql/storm-sql-runtime/src/jvm/org/apache/storm/sql/runtime/datasource/socket/bolt/SocketBolt.java
---
@@ -0,0 +1,108 @@
+/*
+ *
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2443
@srdo
Yes, since Streams API doesn't have rich function like executor side
initialization for now. We could address it but maybe better to file a new
issue and handle it independently. What
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2443#discussion_r202631502
--- Diff: docs/storm-sql-internal.md ---
@@ -1,59 +0,0 @@
--- End diff --
@srdo
Agreed. Regarding updating the doc, I guess
35 matches
Mail list logo
