[GitHub] storm pull request #2908: STORM-3276: Updated Flux to deal with storm local ...
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/2908 ---
[GitHub] storm pull request #2908: STORM-3276: Updated Flux to deal with storm local ...
Github user jnioche commented on a diff in the pull request:
https://github.com/apache/storm/pull/2908#discussion_r237006771
--- Diff: flux/flux-core/src/main/java/org/apache/storm/flux/Flux.java ---
@@ -52,17 +52,22 @@
public class Flux {
private static final Logger LOG = LoggerFactory.getLogger(Flux.class);
+@Deprecated
--- End diff --
Got you! Your comment was pretty clear, I just need another coffee :-)
---
[GitHub] storm pull request #2908: STORM-3276: Updated Flux to deal with storm local ...
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2908#discussion_r237005496
--- Diff: flux/flux-core/src/main/java/org/apache/storm/flux/Flux.java ---
@@ -52,17 +52,22 @@
public class Flux {
private static final Logger LOG = LoggerFactory.getLogger(Flux.class);
+@Deprecated
--- End diff --
Sure, what I meant was if we're going to break the API anyway I think it
makes sense to remove the options entirely, rather than deprecating them.
---
[GitHub] storm pull request #2908: STORM-3276: Updated Flux to deal with storm local ...
Github user jnioche commented on a diff in the pull request:
https://github.com/apache/storm/pull/2908#discussion_r237004334
--- Diff: flux/flux-core/src/main/java/org/apache/storm/flux/Flux.java ---
@@ -52,17 +52,22 @@
public class Flux {
private static final Logger LOG = LoggerFactory.getLogger(Flux.class);
+@Deprecated
--- End diff --
IMHO this it is acceptable for a major release. Projects need a drastic
cleanup once in a while.
---
[GitHub] storm pull request #2908: STORM-3276: Updated Flux to deal with storm local ...
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2908#discussion_r236360412
--- Diff: storm-server/src/main/java/org/apache/storm/LocalCluster.java ---
@@ -323,6 +342,13 @@ private static boolean areAllWorkersWaiting() {
}
}
+private static final long DEFAULT_ZK_PORT = 2181;
--- End diff --
Nit: Please move this to the other constants so it's not in the middle of
method declarations.
---
[GitHub] storm pull request #2908: STORM-3276: Updated Flux to deal with storm local ...
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2908#discussion_r236358832
--- Diff: flux/flux-core/src/main/java/org/apache/storm/flux/Flux.java ---
@@ -52,17 +52,22 @@
public class Flux {
private static final Logger LOG = LoggerFactory.getLogger(Flux.class);
+@Deprecated
--- End diff --
Given that current scripts to start a Flux topology will have to be changed
to make this work, is there any value to preserving these options? If someone
is using one of these, most likely their script will end up failing or doing
the wrong thing.
---
[GitHub] storm pull request #2908: STORM-3276: Updated Flux to deal with storm local ...
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2908#discussion_r234303902
--- Diff: storm-server/src/main/java/org/apache/storm/LocalCluster.java ---
@@ -1197,8 +1238,9 @@ public IBolt makeAckerBoltImpl() {
* When running a topology locally, for tests etc. It is helpful to
be sure that the topology is dead before the test exits. This is
* an AutoCloseable topology that not only gives you access to the
compiled StormTopology but also will kill the topology when it
* closes.
- *
+ * ```
--- End diff --
I generated the javadocs and the markdown plugin didn't work in this case.
I am not sure why, so I cleaned it up.
---
[GitHub] storm pull request #2908: STORM-3276: Updated Flux to deal with storm local ...
Github user kishorvpatil commented on a diff in the pull request:
https://github.com/apache/storm/pull/2908#discussion_r234019467
--- Diff: storm-server/src/main/java/org/apache/storm/LocalCluster.java ---
@@ -1197,8 +1238,9 @@ public IBolt makeAckerBoltImpl() {
* When running a topology locally, for tests etc. It is helpful to
be sure that the topology is dead before the test exits. This is
* an AutoCloseable topology that not only gives you access to the
compiled StormTopology but also will kill the topology when it
* closes.
- *
+ * ```
--- End diff --
Minor one. Not sure if ``` turns it into pre formatted code. Do we need to
use ` tag?
---
[GitHub] storm pull request #2908: STORM-3276: Updated Flux to deal with storm local ...
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2908 STORM-3276: Updated Flux to deal with storm local correctly You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-3276 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2908.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 #2908 commit 150160f534145d568c0368ec19823813ad16136c Author: Robert (Bobby) Evans Date: 2018-11-15T20:12:48Z STORM-3276: Updated Flux to deal with storm local correctly ---
