Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1691
Just to be clear, that wasn't a criticism. I just wanted to point out that
it is important that we know the provenance and license of all code that enters
our repository.
---
If your project
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1691
This a fairly large commit that seemingly includes code from other
projects. That's fine as long as you can document what code was copied, and
what the license for that code was.
---
If your
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1642
I have a few style-related nits, but I've refrained from pointing them out
because the style(s) in the codebase are all over the place, and the style is
inherited in some places. If we want to get
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1642
@revans2 I'm also in the process of reviewing this. I hope to finish by the
end of the day. So far everything looks good to me.
---
If your project is set up for it, you can reply to this email
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1656
Before we get too out of control over white space, we should think about
establishing a style guide.
I don't see any style violations here that we haven't seen before (and
accepted
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1619
+1 nice catch. Those components were added after the last RC, which is why
they weren't included in the revert.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1606
+1. I don't see a need to break them out.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1595
@HeartSaVioR I don't think I was very clear in articulating my proposal.
What I'm saying is rather than making this a configuration switch, make it
dynamic based on what interface the consumer
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/1595#discussion_r72539748
--- Diff: conf/defaults.yaml ---
@@ -259,6 +259,10 @@ topology.disruptor.batch.size: 100
topology.disruptor.batch.timeout.millis: 1
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/1595#discussion_r72513531
--- Diff: conf/defaults.yaml ---
@@ -259,6 +259,10 @@ topology.disruptor.batch.size: 100
topology.disruptor.batch.timeout.millis: 1
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/1595#discussion_r72479602
--- Diff: conf/defaults.yaml ---
@@ -259,6 +259,10 @@ topology.disruptor.batch.size: 100
topology.disruptor.batch.timeout.millis: 1
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1585
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1553
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1583
@omkreddy I haven't had a chance to review it fully yet, but you can add me
as a committer sponsor.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1579
@tibkiss Yes. I tested locally and the build succeeded.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1568
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1569
@darionyaphet This change has the same effect as the code it replaces and
seems unnecessary as there is no synchronization going on here.
---
If your project is set up for it, you can reply
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1579
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1570
@harshach @darionyaphet I don't think we're in a position to deprecate
storm-kafka yet for two reasons:
1. storm-kafka-client does not have feature parity with storm-kafka yet
(e.g. bolt
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1576
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1577
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1578
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1561
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1482
@abhishekagarwal87 Any update on this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1558
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1563
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1560
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1482
The README needs to be updated for this change with instructions on how to
use different Kafka client and scala versions.
I'm okay with either leaving or removing the `provided` scope
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1481
+1 Thanks @abhishekagarwal87.
Since this is a one-line change blocking the release, I'm going to merge it
in the interest of proceeding with the release. If there are any objections we
can
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1325
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1324
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1467
There was a compilation issue with this patch. I fixed it with this commit:
0b54767
Please review. If there are any objections I can revert.
---
If your project is set up for it, you can reply
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1467
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1331
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1331
Two minor nits. I'm +1 once they are addressed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/1331#discussion_r66318933
--- Diff:
external/storm-kafka/src/jvm/org/apache/storm/kafka/ExponentialBackoffMsgRetryManager.java
---
@@ -86,15 +94,23 @@ public Long
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/1331#discussion_r66318510
--- Diff:
external/storm-kafka/src/jvm/org/apache/storm/kafka/SpoutConfig.java ---
@@ -37,6 +37,8 @@
public long retryInitialDelayMs = 0
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1379
I wouldn't mind seeing some larger-scale performance tests, since it
doubles the size of ACK tuples (I know, they're still small). If others can
confirm the absence of any performance degradation
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1453
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1324
See my comments on #1325 as they apply here as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/1325#discussion_r66292563
--- Diff: conf/storm.yaml.example ---
@@ -39,10 +39,13 @@
# - "server2"
## Metrics Consumers
+## NOTE: task queue will be
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/1325#discussion_r66292249
--- Diff: storm-core/src/clj/org/apache/storm/daemon/common.clj ---
@@ -298,18 +299,21 @@
{[comp-id METRICS-STREAM-ID] :shuffle
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1456
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1471
+1 nice job!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1473
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1461
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1153
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1464
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1468
@harshach The source of the file is referenced here:
https://github.com/apache/storm/pull/1468/files#diff-da45fe3972445a9f82ef768808dd8853R20
I'd like to get clearance that what
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1468
> We won't be able capture this in JIRA either. I am not sure how much of
this is important to have all the commits from each contributor for a single
JIRA which in itself is rare unless its a
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1468
> It will ask for primary authors and the user who is merging this can
input more than one author at the time of merge.
That means it removes authorship information. If we tag a squas
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1458
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/1468
I'm a little on the fence in terms of squashing the commits of others vs.
asking the contributor to do so. There are a lot of situations where spreading
out a big patch over multiple commits makes
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1449#issuecomment-01561
+1 This should probably also be applied to the 1.* branches.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1448#issuecomment-222169053
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1446#issuecomment-221764594
+1 Thanks for the clarification @satishd!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1398#issuecomment-221686373
@jerrypeng For tracking what goes into each branch/release. Github only
gives us merge notifications for the branch a pull request targeted. If you had
merged
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/1441#discussion_r64637467
--- Diff: bin/storm.py ---
@@ -241,7 +240,6 @@ def jar(jarfile, klass, *args):
extrajars=[tmpjar, USER_CONF_DIR, STORM_BIN_DIR
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1398#issuecomment-221677136
@jerrypeng Did you merge this to any other branches, or just master?
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/1446#discussion_r64612906
--- Diff: storm-core/src/jvm/org/apache/storm/StormSubmitter.java ---
@@ -223,48 +227,65 @@ public static void submitTopologyAs(String name, Map
stormConf
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/1446#discussion_r64612709
--- Diff: storm-core/src/jvm/org/apache/storm/StormSubmitter.java ---
@@ -223,48 +227,65 @@ public static void submitTopologyAs(String name, Map
stormConf
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/1441#discussion_r64598586
--- Diff: bin/storm.py ---
@@ -241,7 +240,6 @@ def jar(jarfile, klass, *args):
extrajars=[tmpjar, USER_CONF_DIR, STORM_BIN_DIR
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1435#issuecomment-221394681
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1428#issuecomment-221391859
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1426#issuecomment-221390428
Thanks @Darkless012 I filed
https://issues.apache.org/jira/browse/STORM-1862 for this and merged to master,
1.x-branch and 1.0.x-branch.
---
If your project is set up
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1398#issuecomment-221383374
+1
@jerrypeng Can you file a lira for updating the documentation if necessary?
---
If your project is set up for it, you can reply to this email and have your
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1429#issuecomment-221381657
Thanks @whoahbot. I merged this to master, 1.x-branch and 1.0.x-branch.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1393#issuecomment-220705013
Thanks @moesol I merged this into master, 1.x-branch and 1.0.x-branch.
Note that with 1.0.x-branch this created a conflict, as
`KafkaConfig.minFetchByte` had
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1393#issuecomment-220692188
I'm in the process of merging this. Other committers please hold off. More
info to follow...
---
If your project is set up for it, you can reply to this email and have
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1393#issuecomment-220683696
+1 Nice catch!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1426#issuecomment-220682954
+1
@Darkless012 No worries on the branch for this. A committer will merge it
into the appropriate branches.
---
If your project is set up for it, you can
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1423#issuecomment-220677726
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1425#issuecomment-220677854
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1424#issuecomment-220677791
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1432#issuecomment-220676804
Thanks @HeartSaVioR. Merged.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1438#issuecomment-220656428
@lerouxrgd Thanks for the patch, but this looks like a duplicate of #1429
which is eligible for merge. Would you mind closing this PR?
The earlier PR was raised
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1432#issuecomment-22063
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1429#issuecomment-220137390
+1
This should also be merged to the 1.x-branch and master branches.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1419#issuecomment-219501512
another JIRA integration test. Ignore.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1419#issuecomment-219500282
testing JIRA integration, please ignore
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1403#issuecomment-218751708
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1411#issuecomment-218654976
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1383#issuecomment-215865676
+1
Since this is a documentation-only patch, and is a big improvement, I'd
like to pull this into 1.0.1 rc3.
If there are any objections, we can always
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1377#issuecomment-215792468
As this is a very minor change that fixes a blocker, if there are no
objections I will merge it in order to proceed with the 1.0.1 rc3 release.
If there are any
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1368#issuecomment-215784896
=1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1367#issuecomment-215784820
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1366#issuecomment-215784757
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1358#issuecomment-215780240
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1374#issuecomment-215760873
+1
@arunmahadevan I will merge for 1.0.1 rc3.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
GitHub user ptgoetz opened a pull request:
https://github.com/apache/storm/pull/1377
STORM-1741: remove unconditional setting of JAVA_HOME from storm-env.sh
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ptgoetz/storm STORM
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1376#issuecomment-215303147
The patch looks okay to me, but I would like to know why it's necessary. I
can't reproduce the issuue from the branch or rc2 build.
---
If your project is set up
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1361#issuecomment-215184340
+1 (forgot to do so before merging)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1362#issuecomment-215150674
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1351#issuecomment-214497033
Thanks @priyank5485. Merged to master, 1.x-branch and 0.10.x-branch.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1351#issuecomment-213062461
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1297#issuecomment-205493715
+1 (again). Confirmed this fixes the test failures on 1.x-branch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1300#issuecomment-205477201
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1131#issuecomment-204619936
This has been merged into the 1.x branch and will be included in the 1.0
release.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1297#issuecomment-204610812
@satishd For some reason this is causing the storm-core clojure tests to
fail consistently. Could you take a look?
---
If your project is set up for it, you can reply
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1297#issuecomment-204547903
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
201 - 300 of 679 matches
Mail list logo
