Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2941
+1. Please squash, and we can merge once the waiting period is over. I'll
probably wait to merge this until we are done with the current 2.0.0 RC vote.
---
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2941#discussion_r249026503
--- Diff:
external/storm-kafka-monitor/src/main/java/org/apache/storm/kafka/monitor/NewKafkaSpoutOffsetQuery.java
---
@@ -64,27 +64,51 @@ public String
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2940
@MichealShin You don't need to open a new PR. If you push your changes to
the branch this PR is pointing at, the PR will get updated automatically.
That said, I think Github gets weird
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2940#discussion_r248985594
--- Diff:
external/storm-kafka-monitor/src/main/java/org/apache/storm/kafka/monitor/NewKafkaSpoutOffsetQuery.java
---
@@ -75,8 +77,13 @@ public boolean equals
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2940
Looks good. Please squash to one commit.
---
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2939
STORM-3315: Upgrade to Kryo 4
https://issues.apache.org/jira/browse/STORM-3315
Tested compatibility with Kryo 3.0.3 serialization by running the TVL
topology for a couple of minutes with two
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2930
Thanks, looks great.
The positional arguments don't seem to be sorted. I get
```
usage: storm [-h] [--config CONFIG] [-storm_config_opts -c]
{jar,localconf
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2930
The .pyc issue can be solved by adding
`true` to the environment
variables for the two `exec-maven-plugin` executions
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2930
Thanks. It looks great. A few final things:
* When running `mvn clean install -DskipTests`, the tests for the script
are still run. Can we skip the executions if `skipTests` is true
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2937
MINOR: Remove unused parameter from storm-kafka-client-example docs
The kafka_artifact_id parameter is not used anymore.
You can merge this pull request into a Git repository by running:
$ git
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/1550
@bigbang4u2 We bulk closed old unmerged pull requests a while ago. This PR
wasn't approved, and wasn't updated for a long time, so it was closed. If you
want this feature to get merged, yo
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2936
STORM-3312: Upgrade Guava to latest version where possible
https://issues.apache.org/jira/browse/STORM-3312
You can merge this pull request into a Git repository by running:
$ git pull https
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2935
+1
---
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2930#discussion_r245727413
--- Diff: bin/storm.py ---
@@ -296,787 +239,1044 @@ def exec_storm_class(klass, jvmtype="-server",
jvmopts=[], extrajars=[], args=[]
elif
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2930#discussion_r245744213
--- Diff: bin/storm.py ---
@@ -296,787 +239,1044 @@ def exec_storm_class(klass, jvmtype="-server",
jvmopts=[], extrajars=[], args=[]
elif
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2930
Nit: The existing help message prints the commands alphabetically. We might
want to do the same with the new help message. Quick google suggests it is
possible
https://stackoverflow.com/questions
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2930
Can we make running `storm.py` with 0 arguments equivalent to running
`storm.py -h`? Running just `storm.py` gives the following output, which isn't
very nice:
```
PS E:\apache-storm-
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2930
When I run the build, a storm.pyc file is generated in the storm/bin
directory. This file ends up in the storm-dist distribution. I'm guessing this
isn't intentional?
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2930
Would it make sense to run the tests for Python 3 as well as Python 2.7?
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2930
This is in reply to
https://github.com/apache/storm/pull/2930#discussion_r245726485, for some
reason Github won't let me put another comment there.
Makes sense. I think we should at least
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2930#discussion_r245725371
--- Diff: storm-client/pom.xml ---
@@ -240,6 +240,29
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2930#discussion_r245722204
--- Diff: bin/storm.py ---
@@ -296,787 +239,1044 @@ def exec_storm_class(klass, jvmtype="-server",
jvmopts=[], extrajars=[], args=[]
elif
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2930#discussion_r245714702
--- Diff: bin/storm.py ---
@@ -132,13 +56,8 @@ def get_jars_full(adir):
elif os.path.exists(adir):
files = [adir]
-ret
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2930#discussion_r245715647
--- Diff: bin/storm.py ---
@@ -156,49 +95,92 @@ def get_classpath(extrajars, daemon=True,
client=False):
ret.extend(get_wildcard_dir(os.path.join
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2934
I'd recommend looking at the diff via
https://github.com/apache/storm/pull/2934/files?w=1, since the indentation
changes make it hard to tell what changed.
---
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2934
STORM-3310: Make JCQueueTest wait for consumer to read all queued iteâ¦
â¦ms before terminating
https://issues.apache.org/jira/browse/STORM-3310
I've only seen the test fail
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2930#discussion_r245602260
--- Diff: storm-client/pom.xml ---
@@ -240,6 +240,29
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2933
The test passed 100 iterations using `@RepeatedTest`
---
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2933
STORM-3309: Fix flaky tick tuple test
https://issues.apache.org/jira/browse/STORM-3309
I've made the following changes:
* When message timeout is disabled, the acker shouldn'
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2930#discussion_r245480567
--- Diff: storm-client/pom.xml ---
@@ -240,6 +240,29
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2924
Approved as part of https://github.com/apache/storm/pull/2927
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2932
I think the build is failing because requirements.txt is being caught by
the rat check.
The change itself looks fine. I'm not sure it makes sense by itself though,
maybe it should ju
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2929
MINOR: Correct comment about overflow limiting in JCQueue. Setting ovâ¦
â¦erflow limit to 0 disables limiting, does not disable overflow
You can merge this pull request into a Git repository by
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2927#discussion_r242739488
--- Diff:
storm-core/test/jvm/org/apache/storm/integration/TopologyIntegrationTest.java
---
@@ -0,0 +1,927 @@
+/*
+ * Copyright 2018 The Apache
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2927#discussion_r242699069
--- Diff:
storm-core/test/jvm/org/apache/storm/integration/TopologyIntegrationTest.java
---
@@ -0,0 +1,927 @@
+/*
+ * Copyright 2018 The Apache
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2927#discussion_r242698501
--- Diff:
storm-core/test/jvm/org/apache/storm/integration/TopologyIntegrationTest.java
---
@@ -0,0 +1,927 @@
+/*
+ * Copyright 2018 The Apache
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2908
+1.
I'm still wondering why we need to preserve command line options here
https://github.com/apache/storm/pull/2908#discussion_r236358832 but I think
it's fine either way.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2928
The build failed because of a hiccup when downloading dependencies. See
https://travis-ci.org/srdo/storm/builds/468124888 instead.
---
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2928
STORM-3270: Build Storm with Java 11, excluding some incompatible modâ¦
â¦ules
https://issues.apache.org/jira/browse/STORM-3270
All Hadoop-related modules are excluded from tests
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2907
Thanks for the reviews, merged to master.
---
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2927
STORM-1307: Port testing4j_test.clj to Java
Follow up to https://github.com/apache/storm/pull/2924, please review that
one first.
You can merge this pull request into a Git repository by running
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2920
+1, please squash to one commit and I'll merge :)
---
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2924
STORM-1289: Port integration-test.clj to Java
https://issues.apache.org/jira/browse/STORM-1289
I have also ported a few tests to JUnit 5 and changed the IntegrationTest
and PerformanceTest
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2922
The test failure is due to broken stubbing, I'll fix it soon
---
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2923
STORM-3301: Fix case where KafkaSpout could emit tuples that were alrâ¦
â¦eady committed
1.x version of https://github.com/apache/storm/pull/2922
You can merge this pull request into a Git
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2922
STORM-3301: Fix case where KafkaSpout could emit tuples that were alrâ¦
â¦eady committed
https://issues.apache.org/jira/browse/STORM-3301
I removed an unnecessary contains check in
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2921
STORM-3300: Fix NPE in Acker that could occur if sending reset timeouâ¦
â¦t tuples
The error occurs when `int task = curr.spoutTask;` is hit during processing
of a reset timeout tuple, if
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2917
Ok, just wanted to be sure that there wasn't an issue to document.
+1
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2917
Looks good. Could you update the commit message to contain the issue number?
Is this upgrade fixing a known issue?
---
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2919
STORM-3296: Upgrade curator-test to avoid CURATOR-409
https://issues.apache.org/jira/browse/STORM-3296
You can merge this pull request into a Git repository by running:
$ git pull https
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2907
Rebased to fix conflicts and squashed to one commit.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2913
Thanks for reviewing, merged to master.
---
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
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2912
Yes, I think those are the affected versions. I figured people would go
look at the JIRA to see which versions are affected, but I'll list them here as
well.
---
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
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
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2914
Thanks @govind-menon, merged to master.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2914
+1
---
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2913
STORM-3290: Split configuration for storm-kafka-client Trident and noâ¦
â¦n-Trident spout into two classes.
https://issues.apache.org/jira/browse/STORM-3290
This moves configuration
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2911#discussion_r234901127
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java
---
@@ -137,7 +142,11 @@ public KafkaSpoutConfig
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2904
Thanks @chiba3, merged to master.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2904
+1. I think this is akin to a documentation change (formatting only), so
doesn't need an issue filed for it.
---
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2911#discussion_r234541339
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java
---
@@ -137,7 +142,11 @@ public KafkaSpoutConfig
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2911#discussion_r234542042
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/trident/KafkaTridentSpoutEmitter.java
---
@@ -225,7 +229,23 @@ private void
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2911#discussion_r234541619
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/trident/KafkaTridentSpoutEmitter.java
---
@@ -225,7 +229,23 @@ private void
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2907
Added a check to `reEmitPartitionBatch`. Let me know if it looks
reasonable, and I'll squash.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2907
Regarding looping offsets, I don't believe Kafka really does anything
special to handle offset value overflow (quick google turned up
http://mail-archives.apache.org/mod_mbox/kafka-users/201510
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2907#discussion_r234338505
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/trident/KafkaTridentSpoutEmitter.java
---
@@ -170,15 +170,25 @@ public void
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2907#discussion_r233750714
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/trident/KafkaTridentSpoutEmitter.java
---
@@ -218,26 +228,27 @@ private void
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2907
Test failure looks unrelated.
---
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2907
STORM-2990, STORM-3279: Fix issue where Kafka Trident spout could ignâ¦
â¦ore EARLIEST and LATEST, and make EARLIEST and LATEST only take effect
on topology deploy
See https
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2905
STORM-2891: Upgrade checkstyle plugin to 3.0.0
See https://issues.apache.org/jira/browse/STORM-2891
You can merge this pull request into a Git repository by running:
$ git pull https://github.com
Github user srdo closed the pull request at:
https://github.com/apache/storm/pull/2653
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2653
Closing due to lack of interest.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2896
@d2r You can test this by checking out https://github.com/apache/storm-site
and following the guide for publishing the documentation
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2892
+1
---
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2892#discussion_r227063418
--- Diff: docs/Local-mode.md ---
@@ -7,7 +7,9 @@ Local mode simulates a Storm cluster in process and is
useful for developing and
To run a topology
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2890#discussion_r227033367
--- Diff:
integration-test/src/main/java/org/apache/storm/st/topology/TestableTopology.java
---
@@ -17,14 +17,18 @@
package
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2880
+1, thanks for handling this.
---
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2890
STORM-3268: Improve integration test stability
https://issues.apache.org/jira/browse/STORM-3268
Basically the integration test can't handle logs rolling during the test.
To fix this we
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2889
STORM-3267: Disable Java 10 build
https://issues.apache.org/jira/browse/STORM-3267
We can add Java 11 at some later point, but for now we need to disable the
Java 10 build.
You can merge
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2877
Looks great, +1
---
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2877#discussion_r224891745
--- Diff: integration-test/run-it.sh ---
@@ -91,6 +90,11 @@ function start_storm_process() {
echo starting: storm $1
sudo su storm -c "e
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2858
Thanks @d2r, merged to master.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2858
+1, thanks for addressing my comment. Please squash to one commit
containing the issue number.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2858
I'd be fine with just moving them to the new profile. It already contains
the integration test, which also isn't in externals.
---
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2858
This looks good, but I'm wondering if Flux and the sql modules should be
added to externals as well? They depend on storm-kafka-client, so the build
won't work with externals disabled unless
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2840
I'm hoping @zd-project will help review this, since this is a rebase of his
PR. I don't really have the context to know whether these added metrics are
useful.
---
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2840
STORM-3147: Add metrics based on ClusterSummary
Rebase and update of https://github.com/apache/storm/pull/2764.
@zd-project Please take a look when you have a chance.
You can merge this pull
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2836
Looks good, thanks. +1
---
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2836#discussion_r218177587
--- Diff: storm-server/pom.xml ---
@@ -171,7 +171,7 @@
maven-checkstyle-plugin
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2805
Test failure looks unrelated, it's getting a permission error when writing
to the local maven repo.
---
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2805#discussion_r218150464
--- Diff: storm-server/src/main/java/org/apache/storm/LocalDRPC.java ---
@@ -38,9 +39,9 @@
private final DRPC drpc;
private final String
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2805#discussion_r218148492
--- Diff:
storm-server/src/main/java/org/apache/storm/daemon/supervisor/Container.java ---
@@ -219,11 +201,11 @@ public void kill() throws IOException
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2836#discussion_r218142320
--- Diff:
storm-server/src/main/java/org/apache/storm/daemon/nimbus/HeartbeatCache.java
---
@@ -0,0 +1,229 @@
+/*
+ * Licensed to the Apache Software
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2836#discussion_r218121407
--- Diff: storm-server/pom.xml ---
@@ -171,7 +171,7 @@
maven-checkstyle-plugin
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2836#discussion_r217911368
--- Diff:
storm-server/src/main/java/org/apache/storm/daemon/nimbus/HeartbeatCache.java
---
@@ -0,0 +1,229 @@
+/*
+ * Licensed to the Apache Software
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2836#discussion_r217911059
--- Diff: storm-client/src/jvm/org/apache/storm/stats/ClientStatsUtil.java
---
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2836#discussion_r218140315
--- Diff:
storm-server/src/main/java/org/apache/storm/daemon/nimbus/HeartbeatCache.java
---
@@ -0,0 +1,229 @@
+/*
+ * Licensed to the Apache Software
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2836#discussion_r218124022
--- Diff:
storm-server/src/main/java/org/apache/storm/daemon/nimbus/HeartbeatCache.java
---
@@ -0,0 +1,229 @@
+/*
+ * Licensed to the Apache Software
1 - 100 of 2052 matches
Mail list logo
