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/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!
[storm-sql-internal-example.xml.txt](https://github.com/apache/storm/files/2198168/st
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2443
[storm-sql-internal-example.xml(1).txt](https://github.com/
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 i
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 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
addres
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 at
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 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 srdo commented on the issue:
https://github.com/apache/storm/pull/2443
Tried out this branch with a storm-sql-kafka example, it seems to work
fine.
---
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2443
@srdo
Rebased. I'm seeing intermittent test failure like below, but not
consistent failure. Will try to take a look at once I have time to, but let's
move it out of this PR.
```
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2443
@srdo
Yeah, thanks for reminding this.
Applying the change manually again would be easier than rebasing, since
there were nice efforts on storm-sql (checkstyle violation reduction,
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2443
@HeartSaVioR Are you still planning on getting this in? I could probably
put some time in to trying this out if no one else is willing to test/use
storm-sql, so we can get a +1.
---
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2443
Rebased to apply dropping standalone mode.
---
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2443
@srdo Thanks again for the detailed review. Addressed review comments.
---
19 matches
Mail list logo
