[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-23 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/incubator-pirk/pull/93 --- 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] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread ellisonanne
Github user ellisonanne commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r80058520 --- Diff: src/main/resources/META-INF/services/org.apache.pirk.responder.wideskies.spi.ResponderPlugin --- @@ -0,0 +1,5 @@ +org.apache.pirk.re

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread tellison
Github user tellison commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r80055483 --- Diff: src/main/resources/META-INF/services/org.apache.pirk.responder.wideskies.spi.ResponderPlugin --- @@ -0,0 +1,5 @@ +org.apache.pirk.respo

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread ellisonanne
Github user ellisonanne commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r80037087 --- Diff: src/main/resources/META-INF/services/org.apache.pirk.responder.wideskies.spi.ResponderPlugin --- @@ -0,0 +1,5 @@ +org.apache.pirk.re

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread tellison
Github user tellison commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r80030927 --- Diff: src/main/java/org/apache/pirk/responder/wideskies/ResponderDriver.java --- @@ -50,103 +40,31 @@ { private static final Logger lo

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread DarinJ
Github user DarinJ commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r80029052 --- Diff: src/main/resources/META-INF/services/org.apache.pirk.responder.wideskies.spi.ResponderPlugin --- @@ -0,0 +1,5 @@ +org.apache.pirk.respond

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread DarinJ
Github user DarinJ commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r80026414 --- Diff: src/main/java/org/apache/pirk/responder/wideskies/spi/ResponderPlugin.java --- @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Softwar

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread DarinJ
Github user DarinJ commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r80026380 --- Diff: src/main/java/org/apache/pirk/responder/wideskies/ResponderDriver.java --- @@ -50,103 +40,31 @@ { private static final Logger logg

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread DarinJ
Github user DarinJ commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r80026462 --- Diff: src/main/resources/responder.properties --- @@ -27,9 +27,15 @@ pir.dataInputFormat= #outputFile -- required -- Fully qualified name of out

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread DarinJ
Github user DarinJ commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r80025462 --- Diff: src/main/java/org/apache/pirk/responder/wideskies/spark/SparkResponder.java --- @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Softwa

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread Tim Ellison
On 22/09/16 10:40, Ellison Anne Williams wrote: > Why are we using the service-plugin model when we are dropping the platform > designations? I'm not sure what "dropping the platform designations" means. Clearly we need different responders for different backends, and I'm guessing we need to tell

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread tellison
Github user tellison commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r80012564 --- Diff: src/main/resources/META-INF/services/org.apache.pirk.responder.wideskies.spi.ResponderPlugin --- @@ -0,0 +1,5 @@ +org.apache.pirk.respo

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread Ellison Anne Williams
Why are we using the service-plugin model when we are dropping the platform designations? Thought that we were going to drop all central platform awareness from Pirk in favor of the ResponderLauncher. The service-plugin model retains it in a roundabout way as we have to statically maintain the list

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread ellisonanne
Github user ellisonanne commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r80005357 --- Diff: src/main/resources/META-INF/services/org.apache.pirk.responder.wideskies.spi.ResponderPlugin --- @@ -0,0 +1,5 @@ +org.apache.pirk.re

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread ellisonanne
Github user ellisonanne commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r80003616 --- Diff: src/main/java/org/apache/pirk/responder/wideskies/spi/ResponderPlugin.java --- @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache So

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread tellison
Github user tellison commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r79989030 --- Diff: src/main/java/org/apache/pirk/responder/wideskies/standalone/StandaloneResponder.java --- @@ -0,0 +1,58 @@ +/* + * Licensed to the

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread tellison
Github user tellison commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r79988418 --- Diff: src/main/java/org/apache/pirk/responder/wideskies/ResponderDriver.java --- @@ -50,103 +40,31 @@ { private static final Logger lo

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread tellison
Github user tellison commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r79988842 --- Diff: src/main/java/org/apache/pirk/responder/wideskies/spark/SparkResponder.java --- @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Soft

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread tellison
Github user tellison commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r79987737 --- Diff: src/main/java/org/apache/pirk/responder/wideskies/ResponderService.java --- @@ -0,0 +1,73 @@ +package org.apache.pirk.responder.wideskie

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-22 Thread tellison
Github user tellison commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r79988975 --- Diff: src/main/java/org/apache/pirk/responder/wideskies/spi/ResponderPlugin.java --- @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Softw

Re: Google docs (was: Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE)

2016-09-21 Thread Suneel Marthi
On Wed, Sep 21, 2016 at 10:43 AM, Tim Ellison wrote: > On 21/09/16 02:40, Darin Johnson wrote: > > Suneel, a google doc as promised, only a day late (sorry - sick kid). > > > > https://docs.google.com/document/d/1K8E0TridC1hBfqDwWCqdZ8Dj_5_ > mMrRQyynQ-Q6MFbI/edit?usp=sharing > > > > I was planni

Google docs (was: Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE)

2016-09-21 Thread Tim Ellison
On 21/09/16 02:40, Darin Johnson wrote: > Suneel, a google doc as promised, only a day late (sorry - sick kid). > > https://docs.google.com/document/d/1K8E0TridC1hBfqDwWCqdZ8Dj_5_mMrRQyynQ-Q6MFbI/edit?usp=sharing > > I was planning on working on this, but I'm going to take a day or two to > let o

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-21 Thread Tim Ellison
On 21/09/16 03:22, Ellison Anne Williams wrote: > I am in favor of breaking out pirk-core as specified so that our initial > submodule structure would be as follows: > > pirk-core (encryption,query, inputformat, serialization, utils) > > pirk-responder (core responder incl. standalone) > > pirk-

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-20 Thread Suneel Marthi
A potential use case (please correct me here) could be that someone needs a Storm impl of Responder which means we need to generate an artifact like - 'pirk-responder-storm.jar' On Wed, Sep 21, 2016 at 5:41 AM, Darin Johnson wrote: > So along these lines I want to ask one more set of questions

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-20 Thread Darin Johnson
So along these lines I want to ask one more set of questions. Since storm/spark/hadoop are just responders do we want to put them as modules below responder? I'm not in favor of this but thought I'd ask. Does it make sense to put some responders in a contrib section? Storm does this for a lot of

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-20 Thread Ellison Anne Williams
I am in favor of breaking out pirk-core as specified so that our initial submodule structure would be as follows: pirk-core (encryption,query, inputformat, serialization, utils) pirk-responder (core responder incl. standalone) pirk-querier pirk-storm pirk-mapreduce pirk-spark pirk-benchmark

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-20 Thread Darin Johnson
Suneel, a google doc as promised, only a day late (sorry - sick kid). https://docs.google.com/document/d/1K8E0TridC1hBfqDwWCqdZ8Dj_5_mMrRQyynQ-Q6MFbI/edit?usp=sharing I was planning on working on this, but I'm going to take a day or two to let others comment. Darin On Mon, Sep 19, 2016 at 5:07

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-20 Thread Tim Ellison
On 19/09/16 22:10, Suneel Marthi wrote: > Here's an example from the Flink project for how they go about new features > or system breaking API changes, we could start a similar process. The Flink > guys call these FLIP (Flink Improvement Proposal) and Kafka community > similarly has something calle

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-20 Thread Tim Ellison
On 19/09/16 21:22, Darin Johnson wrote: > Alright, that was in the spirit of what I was thinking when I did this. > > Why don't we take Tim's suggested improvements to my PR (I'll do the > necessary cleanup) and at the same time just remove the platform argument > altogether since backwards compat

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Darin Johnson
Great will write up the doc link here, finish pirk63 then start this. On Sep 19, 2016 5:34 PM, "Suneel Marthi" wrote: > +100 > > On Mon, Sep 19, 2016 at 11:24 PM, Ellison Anne Williams < > eawilli...@apache.org> wrote: > > > Yes, ES is just an inputformat (like HDFS, Kafka, etc) - we don't need

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Darin Johnson
So my concern with elastic search isn't really that I want it as a submodule it's more that I don't think it belongs in any of the jars. Otherwise one can argue every inputformat belongs. It seems more prudent to have those included via some abstraction devs can add on thier own (cascading's tap c

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Suneel Marthi
+100 On Mon, Sep 19, 2016 at 11:24 PM, Ellison Anne Williams < eawilli...@apache.org> wrote: > Yes, ES is just an inputformat (like HDFS, Kafka, etc) - we don't need a > separate submodule. > > Aside from pirk-core, it seems that we would want to break the responder > implementations out into sub

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Ellison Anne Williams
Yes, ES is just an inputformat (like HDFS, Kafka, etc) - we don't need a separate submodule. Aside from pirk-core, it seems that we would want to break the responder implementations out into submodules. This would leave us with something along the lines of the following (at this point): pirk-core

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Suneel Marthi
Here's an example from the Flink project for how they go about new features or system breaking API changes, we could start a similar process. The Flink guys call these FLIP (Flink Improvement Proposal) and Kafka community similarly has something called KLIP. We could start a PLIP (??? :-) ) https

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Ellison Anne Williams
Sounds good. On Mon, Sep 19, 2016 at 4:22 PM, Darin Johnson wrote: > Alright, that was in the spirit of what I was thinking when I did this. > > Why don't we take Tim's suggested improvements to my PR (I'll do the > necessary cleanup) and at the same time just remove the platform argument > alto

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Darin Johnson
Sure will do tonight. On Sep 19, 2016 5:07 PM, "Suneel Marthi" wrote: > A shared Google doc would be more convenient than a bunch of Jiras. Its > easier to comment and add notes that way. > > > On Mon, Sep 19, 2016 at 10:38 PM, Darin Johnson > wrote: > > > Suneel, I'll try to put a couple jiras

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Suneel Marthi
A shared Google doc would be more convenient than a bunch of Jiras. Its easier to comment and add notes that way. On Mon, Sep 19, 2016 at 10:38 PM, Darin Johnson wrote: > Suneel, I'll try to put a couple jiras on it tonight with my thoughts. > Based off my pirk-63 I was able to pull spark and s

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Suneel Marthi
Pirk-ES is more of a Source input (to be more technically precise) so it doesn't warrant being a spearate module of its own. But I am not sure about pirk-hadoop and pirk-storm aren't they both different responder impls (??) ElasticSearch should just be an input source like HDFS and doesn't need t

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Darin Johnson
Suneel, I'll try to put a couple jiras on it tonight with my thoughts. Based off my pirk-63 I was able to pull spark and storm out with no issues. I was planning to pull them out, then tackling elastic search, then hadoop as it's a little entrenched. This should keep most PRs to manageable chunks

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Darin Johnson
Alright, that was in the spirit of what I was thinking when I did this. Why don't we take Tim's suggested improvements to my PR (I'll do the necessary cleanup) and at the same time just remove the platform argument altogether since backwards compatibility isn't upsetting anyone. We'll still need

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Suneel Marthi
Refactor is definitely a first priority. Is there a design/proposal draft that we could comment on about how to go about refactoring the code. I have been trying to keep up with the emails but definitely would have missed some. On Mon, Sep 19, 2016 at 6:57 PM, Ellison Anne Williams < eawilli..

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Ellison Anne Williams
Agree - let's leave the config/CLI the way it is for now and tackle that as a subsequent design discussion and PR. Also, I think that we should leave the ResponderDriver and the ResponderProps alone for this PR and push to a subsequent PR (once we decide if and how we would like to delegate each).

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Tim Ellison
On 19/09/16 15:46, Darin Johnson wrote: > Hey guys, > > Thanks for looking at the PR, I apologize if it offended anyone's eyes:). > > I'm glad it generated some discussion about the configuration. I didn't > really like where things were heading with the config. However, didn't > want to create

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Darin Johnson
Hey guys, Thanks for looking at the PR, I apologize if it offended anyone's eyes:). I'm glad it generated some discussion about the configuration. I didn't really like where things were heading with the config. However, didn't want to create to much scope creep. I think any hierarchical config

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Tim Ellison
On 19/09/16 13:40, Ellison Anne Williams wrote: > It seems that it's the same idea as the ResponderLauncher with the service > component added to maintain something akin to the 'platform'. I would > prefer that we just did away with the platform notion altogether and make > the ResponderDriver 'dum

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Tim Ellison
On 19/09/16 13:39, Suneel Marthi wrote: > The way this PR is now is so similar to how bad IBM SystemML which is a > hackwork of hurriedly put together and something I have often pointed out > to others as a clear example of "how not to design crappy software". See > this gist of an example code sn

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Ellison Anne Williams
Ok - so, I'm officially shamed ;) I'm not a big fan of java CLI either as it tends to be heavyweight and inflexible - it was just super fast to put together as a first pass. Happy to consider other more flexible options... :) On Mon, Sep 19, 2016 at 8:40 AM, Ellison Anne Williams < eawilli...@apa

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Ellison Anne Williams
It seems that it's the same idea as the ResponderLauncher with the service component added to maintain something akin to the 'platform'. I would prefer that we just did away with the platform notion altogether and make the ResponderDriver 'dumb'. We get around needing a platform-aware service by re

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Suneel Marthi
The way this PR is now is so similar to how bad IBM SystemML which is a hackwork of hurriedly put together and something I have often pointed out to others as a clear example of "how not to design crappy software". See this gist of an example code snippet from IBM SystemML - https://gist.github.co

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Tim Ellison
How about an approach like this? https://github.com/tellison/incubator-pirk/tree/pirk-63 The "on-ramp" is the driver [1], which calls upon the service to find a plug-in [2] that claims to implement the required platform responder, e.g. [3]. The list of plug-ins is given in the provider's JAR f

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread DarinJ
Github user DarinJ commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r79377189 --- Diff: src/main/java/org/apache/pirk/responder/wideskies/ResponderDriver.java --- @@ -49,83 +41,111 @@ public class ResponderDriver {

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread DarinJ
Github user DarinJ commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r79377024 --- Diff: src/main/java/org/apache/pirk/responder/wideskies/ResponderDriver.java --- @@ -49,83 +41,111 @@ public class ResponderDriver {

Re: [GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread Tim Ellison
Darin, Unless I'm reading this wrong, the patch still has many references from the ResponderDriver to the set of currently supported responders. This code will have to change when somebody wants to add a new responder type. I thought the plan was to have the responder driver agnostic of the resp

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread tellison
Github user tellison commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r79352002 --- Diff: src/main/java/org/apache/pirk/responder/wideskies/ResponderDriver.java --- @@ -49,83 +41,111 @@ public class ResponderDriver {

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-19 Thread tellison
Github user tellison commented on a diff in the pull request: https://github.com/apache/incubator-pirk/pull/93#discussion_r79351660 --- Diff: src/main/java/org/apache/pirk/responder/wideskies/ResponderDriver.java --- @@ -49,83 +41,111 @@ public class ResponderDriver {

[GitHub] incubator-pirk pull request #93: WIP-Pirk 63-DO NOT MERGE

2016-09-18 Thread DarinJ
GitHub user DarinJ opened a pull request: https://github.com/apache/incubator-pirk/pull/93 WIP-Pirk 63-DO NOT MERGE This is a WIP for [PIRK-63](https://issues.apache.org/jira/browse/PIRK-63) to open the door to other responders without having to modify the actual code of Pirk. It'