Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/991#issuecomment-172727308
@arunmahadevan Thanks. Yes, the revans2 commit is due to upmerge. I'm happy
to squash some commits, but some are there to facilitate merging to multiple
branches
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1010#issuecomment-171780642
+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/1012#issuecomment-171793561
Not really specific to this pull request, but I missed it before when the
profiling functionality was originally added.
If profiling is enabled, we turn
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1011#issuecomment-171796975
Personally, I don't think we need separate pull requests for each branch
yet (master and 1.x have not diverged enough to cause conflicts), as it kind of
confuses things
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1101#issuecomment-183139018
+1
I haven't looked at the conflicts yet, But we may need separate pull
requests for the mast and 1.x branches. But that should be trivial.
Can you add
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1049#issuecomment-175378568
+1
For now I'd say it should be applied to the 1.x branch. Master should be
considered as well, since the .clj files involved haven't really come into play
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/1050#discussion_r51036546
--- Diff:
storm-core/src/jvm/org/apache/storm/trident/planner/processor/MapProcessor.java
---
@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1052#issuecomment-178734184
Looks good. +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
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1056#issuecomment-178288859
There's been no response from the submitter, and there's not a clear
definition of what this aims to address.
I'm +1 for closing this.
---
If your project
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1050#issuecomment-178283234
@arunmahadevan Can we move the peek implementation to a separate discussion
so we can discuss it independently?
And can you overload the filter() method
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1050#issuecomment-177022612
@arunmahadevan lets go with the each() based approach for now since that's
more familiar to existing users.
Also can you file a follow up jira or update
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1017#issuecomment-179493698
+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/1038#issuecomment-176912782
+1
I've not used MongoDB in ages, so I'm in the same boat as @revans2. Are
there any other committers that are willing to sponsor this so it is maintained
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1050#issuecomment-178707222
+1 Thanks @arunmahadevan
---
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/1052#discussion_r51607023
--- Diff: external/storm-hdfs/README.md ---
@@ -315,6 +314,18 @@ An `org.apache.avro.Schema` object cannot be directly
provided since it does
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1052#issuecomment-178722021
One minor issue with the markdown formatting in the README, but I'm +1 once
that's fixed.
---
If your project is set up for it, you can reply to this email and have
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/1050#discussion_r51045778
--- Diff:
storm-core/src/jvm/org/apache/storm/trident/operation/FlatMapFunction.java ---
@@ -0,0 +1,36 @@
+/**
+ * Licensed to the Apache Software
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1016#issuecomment-172988140
@jerrypeng Can you add this to the CHANGELOG?
---
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/963#issuecomment-173655683
Still +1. Since there was a recent code change we should hold off on
merging until there's been enough time for review.
---
If your project is set up for it, you can
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1029#issuecomment-174062054
@revans2 Yes, please.
---
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 opened a pull request:
https://github.com/apache/storm/pull/1029
STORM-1214: add javadoc for Trident Streams and Operations
JIRA: https://issues.apache.org/jira/browse/STORM-1214
This is just a work in progress for STORM-1214, if this is merged, please
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1015#issuecomment-173314105
+1 (and +1 for merging to 1.x 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
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/1029#discussion_r50288291
--- Diff:
storm-core/src/jvm/org/apache/storm/trident/operation/Operation.java ---
@@ -20,7 +20,26 @@
import java.io.Serializable;
import
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1023#issuecomment-173310578
+1
I wouldn't mind seeing this go back to 1.x and 0.10.x (0.10.x has a few
other minor fixes). But I don't think we need to support 0.9.x too much longer
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1029#issuecomment-173290288
Sorry for the package renaming noise. I missed a commit.
---
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 commented on the pull request:
https://github.com/apache/storm/pull/1027#issuecomment-173344407
One very minor nit.
+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
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/1027#discussion_r50310048
--- Diff: storm-core/src/jvm/org/apache/storm/messaging/netty/Login.java ---
@@ -0,0 +1,411 @@
+/**
+ * Licensed to the Apache Software Foundation
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/991#issuecomment-173357623
@Parth-Brahmbhatt Done.
---
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/1023#issuecomment-173345063
@revans2 I'll take care of the merge to 0.10.x as soon as this is in master.
---
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/991#issuecomment-173353051
@Parth-Brahmbhatt Good catch. Will do.
---
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 opened a pull request:
https://github.com/apache/storm/pull/1032
STORM-1452: update REST API docs for profiling
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ptgoetz/storm asf-site-ptgoetz2
Alternatively
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1027#issuecomment-173359428
Still +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
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1012#issuecomment-173361855
@d2r You can just delete the .md file in question. I should have a pull
request for the docs in a minute.
---
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/1012#issuecomment-173363553
PR for moving the docs to `asf-site`:
https://github.com/apache/storm/pull/1032
That includes the typo @revans2 mentioned.
---
If your project is set up
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1016#issuecomment-173382241
+1 for backporting to 1.x-branch.
---
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/1032#issuecomment-173380332
As this is a documentation-only patch, and was previously reviewed as part
of #1012, I'm going to merge this. If there are any objections, we can revert
the change
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1112#issuecomment-184905183
+0 at this time just because I'd like to do some additional testing and
verification.
---
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/1127#issuecomment-186369719
+1 one very minor comment
---
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/1122#issuecomment-186370134
+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/1115#issuecomment-186370862
+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/1112#issuecomment-186371004
+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/1079#issuecomment-186372728
+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/1087#issuecomment-186373734
needs another upmerge.
---
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/1076#issuecomment-186386735
+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/1122#issuecomment-186459994
@jnioche yes, this can go into 1.0, all you need to do is request so (which
you have). No need to open a second pull request unless it doesn't apply
cleanly to the 1.x
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1050#issuecomment-178592968
@arunmahadevan Yes, I'm talking about adding the following:
```java
public Stream filter(Fields inputFields, Filter filter)
```
I know it's
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1190#issuecomment-194392972
+1 once @revans2's thread safety concern is addressed.
We may want to document that this feature requires ack'ing to be enabled,
but that can be handled
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1130#issuecomment-197018229
+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/1206#issuecomment-197014240
+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/1210#issuecomment-197013148
+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/1208#issuecomment-196987610
+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/1208#issuecomment-196989428
Merged to master and 1.x-branch.
---
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/1199#issuecomment-198068879
@knusbaum Yeah, you're right. :) I always try to wait based on last patch.
We should think about changing that.
As far as pulling it into 1.x, I am +1
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1206#issuecomment-198048775
Merged to 1.x-branch.
---
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/1190#issuecomment-196447926
+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/1199#issuecomment-196585455
I lean toward making this part of the Stream API, just like
parallelismHint(). In both cases it is about requesting cluster resources,
(I.e. Threads/CPU/Memory
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1221#issuecomment-198042279
+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/1219#issuecomment-197404109
+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/1215#issuecomment-198044924
+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/1214#issuecomment-198045546
+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/871#issuecomment-202559608
+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/1217#issuecomment-202556630
It should be fairly easy to alter the UI to make it clear to users when
this is disabled, as well as how to enable it. If we can do that I would
support disabling
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1044#issuecomment-203612885
+1. We should also merge this into 1.x-branch.
---
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/1275#issuecomment-203602471
+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/1044#issuecomment-203621639
@dossett Good point. We may want to hold off on 1.x since that change is
backward incompatible.
---
If your project is set up for it, you can reply to this email
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1270#issuecomment-203603553
+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/1269#issuecomment-203605038
@arunmahadevan That's likely a typo or formatting error in the markdown
file.
---
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/1131#issuecomment-203633842
+1 for 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 not have
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1217#issuecomment-203691155
@roshannaik, thank you for your patience, persistence, and dedication
throughout the lifecycle of this patch. I hate to vote -1 on anything. I'm
really glad we're
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1131#issuecomment-203693035
I think this wins the record for the most comments on a pull request.
Thanks @hmcl for persevering and putting up with the epic review. Nice work.
---
If your
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1199#issuecomment-198072271
@knusbaum Looks like the clojure tests diverged, I haven't looked at how
much. When I think about trident I usually think "not clojure" and forget about
the i
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1228#issuecomment-198039817
+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/1199#issuecomment-198052573
+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/1199#issuecomment-198074964
@knusbaum Yeah, just looked at the conflicts. Not to bad.
Go for it.
---
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/1240#issuecomment-14152
+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/1202#issuecomment-11867
+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/1239#issuecomment-199989200
@revans2 The `storm sql` command (via the `StormSql` class) builds a fat
topology jar based on the class path, and submits it.
I have some concerns around
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1220#issuecomment-20085
+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/1239#issuecomment-200529436
I'm +1 for merging this. I think moving the jars out of the lib folder is
the most important part for the 1.0 release. How to package the SQL
dependencies for the best
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/1072#discussion_r57232035
--- Diff:
external/storm-hbase/src/main/java/org/apache/storm/hbase/trident/windowing/HBaseWindowsStore.java
---
@@ -0,0 +1,275 @@
+/**
+ * Licensed
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1223#issuecomment-200521136
+1
Tested on a real cluster and compared to 1.x-branch before and after this
patch. Throughput increased and latency decreased.
---
If your project is set up
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1217#issuecomment-200591500
We're debating six versus one half dozen. Do we disable it by default and
explicitly tell users they have to turn it on for the UI functionality to work?
Or do we
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1222#issuecomment-198042109
+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/1254#issuecomment-201097134
+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/1217#issuecomment-201105956
Yes, there is a performance hit. Especially in a single node cluster (don't
know if you were running multiple workers), and networking doesn't come into
play . I'm
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1217#issuecomment-201120771
@hararha Keep in mind that anyone packaging Apache Storm is free to make
changes.
---
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/1168#issuecomment-193907421
+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/1087#issuecomment-192370725
+1 for merging to both branches.
---
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/1188#issuecomment-193979600
This doesn't compile, as @abellina pointed out. Adding back the parens on
line 1337 fixed the compilation error and all tests pass.
---
If your project is set up
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1179#issuecomment-193971785
+1. The travis failures seem like a red herring. All tests passed in my
environment.
---
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/1194#issuecomment-193972568
+1. We should also apply this to 1.x-branch.
---
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/1190#issuecomment-193934901
@arunmahadevan Can you document the behavior of non-stateful bolts in a
topology with stateful bolts?
In a traditional topology (i.e. one that does not include
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1292#issuecomment-204468688
+1
As this is only a configuration change (not a code change) I intend to
merge it before the 24 hr. waiting period closes.
---
If your project is set up
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1285#issuecomment-204472891
+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/1299#issuecomment-204540722
@zhuoliu Yep, just caught that now and applied. Running tests and will
commit shortly.
---
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/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
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/1273#issuecomment-203507676
+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/1281#issuecomment-204034904
+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/1284#issuecomment-204039770
+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/1273#issuecomment-204110947
@satishd I need to revert the merge because of compilation issues.
---
If your project is set up for it, you can reply to this email and have your
reply appear
301 - 400 of 679 matches
Mail list logo
