Github user danny0405 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193434753
--- Diff:
storm-client/src/jvm/org/apache/storm/messaging/netty/SaslStormClientHandler.java
---
@@ -41,80 +38,88 @@ public
Github user danny0405 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193438481
--- Diff:
storm-server/src/main/java/org/apache/storm/pacemaker/PacemakerServer.java ---
@@ -83,48 +88,51 @@ public PacemakerServer(IServerMessageHandler
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2698
Thanks for addressing the comments. Looks good to me. I think it makes
sense to add shaded-deps as a module to the root pom though. It will likely
address the release issue, and means we don't have to
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193462736
--- Diff: storm-core/pom.xml ---
@@ -365,17 +365,17 @@
${basedir}/src/resources
-
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193462962
--- Diff: storm-core/pom.xml ---
@@ -456,29 +456,29 @@
- org.apache.storm
-
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193463579
--- Diff:
storm-client/src/jvm/org/apache/storm/pacemaker/codec/ThriftEncoder.java ---
@@ -12,59 +12,54 @@
package
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193452701
--- Diff: storm-client/src/jvm/org/apache/storm/messaging/netty/Client.java
---
@@ -135,19 +141,29 @@
Github user danny0405 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193438066
--- Diff: storm-core/pom.xml ---
@@ -456,29 +456,29 @@
- org.apache.storm
-
Github user danny0405 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193437756
--- Diff: storm-core/pom.xml ---
@@ -140,7 +140,7 @@
data.codec
test
-
+
Github user danny0405 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193437909
--- Diff: storm-core/pom.xml ---
@@ -365,17 +365,17 @@
${basedir}/src/resources
-
Github user danny0405 commented on the issue:
https://github.com/apache/storm/pull/2703
@revans2
You are right, i just thought if we can keep all these clients sync in
rule, but fail fast just for this is ok.
---
Github user danny0405 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193436151
--- Diff:
storm-client/src/jvm/org/apache/storm/messaging/netty/StormServerPipelineFactory.java
---
@@ -12,28 +12,40 @@
package
Github user danny0405 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193437371
--- Diff: storm-client/src/jvm/org/apache/storm/utils/TransferDrainer.java
---
@@ -40,94 +41,39 @@ public void add(TaskMessage taskMsg) {
}
Github user danny0405 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193437501
--- Diff: storm-client/src/jvm/org/apache/storm/utils/TransferDrainer.java
---
@@ -40,94 +41,39 @@ public void add(TaskMessage taskMsg) {
}
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2703
I agree it would be great to keep all of the clients in sync, but the java
is the only high level client that we support. The only other client we
generate the code for is a python client, but it
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193462295
--- Diff:
storm-server/src/main/java/org/apache/storm/pacemaker/PacemakerServer.java ---
@@ -83,48 +88,51 @@ public PacemakerServer(IServerMessageHandler
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2703
@danny0405 This is just a performance/usability optimization. The checks
are still happening on the server side, so the other clients will still get an
exception when they try to upload a jar, just
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2698
@srdo I can add shaded-deps to the root pom, but it is going to potentially
mask a lot of build issues, and is going to make IDEs confused.
maven-3.2.6 and above added in a cache for
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193481874
--- Diff: storm-client/src/jvm/org/apache/storm/messaging/netty/Client.java
---
@@ -135,19 +141,29 @@
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2704
I ran some performance tests, but they were on my laptop with other things
going on in the background so all I can really tell is that the 4.x code is
withing noise for 3.x. But the margin for
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193493524
--- Diff:
storm-client/src/jvm/org/apache/storm/pacemaker/codec/ThriftEncoder.java ---
@@ -12,59 +12,54 @@
package org.apache.storm.pacemaker.codec;
Hello Apache Supporters and Enthusiasts
This is a final reminder that our Apache EU Roadshow will be held in
Berlin next week on 13th and 14th June 2018. We will have 28 different
sessions running over 2 days that cover some great topics. So if you are
interested in Microservices, Internet of
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/2698
+1
@srdo You asked what the maven release commands were. They are:
```
mvn release:prepare -P dist
mvn release:perform -P dist
```
Note that the process modifies
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2698
@revans2 If I'm understanding the problem correctly, the bug is that if
shaded-deps is in the same reactor as the other modules, Maven will read the
"normal" pom, rather than the dependency reduced
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2698
@srdo It might work to make them optional. I am not a maven expert when it
comes to that..., or really just about anything with maven beyond setting up a
basic build. I am happy to try it out, as
GitHub user revans2 opened a pull request:
https://github.com/apache/storm/pull/2707
STORM-3097: Remove storm-druid
This is the follow on to #2706 which deprecates storm-druid for 1.x
releases.
You can merge this pull request into a Git repository by running:
$ git pull
GitHub user revans2 opened a pull request:
https://github.com/apache/storm/pull/2706
STORM-3097: deprecate storm-druid (1.x)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/revans2/incubator-storm STORM-3097-deprecate
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2706
+1, but please also add the note to
https://github.com/apache/storm/blob/1.x-branch/docs/storm-druid.md
---
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2706
This can be cherry-picked into 1.1.x, but 1.0.x does not even support
storm-druid.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2707
Remember https://github.com/apache/storm/blob/master/docs/storm-druid.md
and any links in the other docs referring to druid. +1 otherwise.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2704
One of the tests is flaky, currently trying to figure out why.
https://travis-ci.org/apache/storm/jobs/388836374. The client is reporting that
it's connected before channelActive has been called on the
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2698
Thanks @ptgoetz
---
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2698
@srdo your suggestion to mark the dependencies as optional in the shaded
pom was a great one. I have it working now where shaded-deps is built by
default, maven does not include any of the unwanted
Github user danny0405 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193436981
--- Diff:
storm-client/src/jvm/org/apache/storm/pacemaker/PacemakerClient.java ---
@@ -100,23 +99,25 @@ public PacemakerClient(Map config,
String host) {
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2703
+1, it seems fine to me to duplicate the check for convenience
---
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2704#discussion_r193461968
--- Diff:
storm-client/src/jvm/org/apache/storm/pacemaker/PacemakerClient.java ---
@@ -100,23 +99,25 @@ public PacemakerClient(Map config,
String host) {
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2698
Looks great. +1.
---
37 matches
Mail list logo
