[jira] [Commented] (PIRK-63) Generalize ResponderDriver to use accept a RespondLauncher class
[
https://issues.apache.org/jira/browse/PIRK-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15502817#comment-15502817
]
ASF GitHub Bot commented on PIRK-63:
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
{
private static final Logger logger =
LoggerFactory.getLogger(ResponderDriver.class);
+ // ClassNames to instantiate Platforms using the platform CLI
+ private final static String MAPREDUCE_LAUNCHER =
"org.apache.pirk.responder.wideskies.mapreduce.MapReduceResponderLauncher";
+ private final static String SPARK_LAUNCHER =
"org.apache.pirk.responder.wideskies.spark.SparkResponderLauncher";
+ private final static String SPARKSTREAMING_LAUNCHER =
"org.apache.pirk.responder.wideskies.spark.streaming.SparkStreamingResponderLauncher";
+ private final static String STANDALONE_LAUNCHER =
"org.apache.pirk.responder.wideskies.standalone.StandaloneResponderLauncher";
+ private final static String STORM_LAUNCHER =
"org.apache.pirk.responder.wideskies.storm.StormResponderLauncher";
--- End diff --
I'm confused by this, I though the goal of PIRK-63 was to avoid having to
change the ResponderDriver each time a new responder type is introduced?
> Generalize ResponderDriver to use accept a RespondLauncher class
>
>
> Key: PIRK-63
> URL: https://issues.apache.org/jira/browse/PIRK-63
> Project: PIRK
> Issue Type: Improvement
> Components: Responder
>Affects Versions: 0.1.0
>Reporter: DarinJ
>Assignee: DarinJ
>Priority: Minor
> Fix For: 0.0.1
>
>
> Currently as discussed on the mailing list, the ResponderDriver uses a switch
> statement based off a string in the CLI or properties file to determine the
> framework to launch. This is require a code change to the ResponderDriver
> for every framework added.
> The solution is to create a ResponderLauncher Interface which each framework
> overrides.
> {quote}
> public interface ResponderLauncher {
> public run()
> }
> {quote}
> To do this, we the change the string specifying the framework to the name of
> the class implementing ResponderLauncher i.e. SparkResponderLauncher and
> instantiate via reflection.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (PIRK-63) Generalize ResponderDriver to use accept a RespondLauncher class
[
https://issues.apache.org/jira/browse/PIRK-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15502410#comment-15502410
]
ASF GitHub Bot commented on PIRK-63:
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's submitted for feedback only, please DO NOT MERGE. I've only
tested standalone mode.
It deprecates the "platform" CLI option in favor of the "launcher" option
which is the name of a class implementing the `ResponderLauncher` interface
which will invoke the run method via reflection. This allows a developer of a
different responder to merely place a jar on the classpath and specify the
appropriate `ResponderLauncher` on the classpath.
The "platform" CLI option is still made available. However, I removed the
explicit dependencies in favor of using reflection. This was done in
anticipation other refactoring the build into submodules, though this does
admittedly make the code more fragile.
ResponderDriver had no unit tests, and unfortunately I saw no good way to
create good ones for this particular change, especially as it required multiple
frameworks to run.
I should say that another possible route here is to have each framework
responder implement their own ResponderDriver. We could provide some utilities
to check the minimum Pirk required options are set, but leave the rest to the
implementation of the responder. It would clean up the ResponderCLI and
ResponderProps which are rather bloated and might continue to grow if left
unchecked.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/DarinJ/incubator-pirk Pirk-63
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-pirk/pull/93.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #93
commit dda458bb2ae77fd9e3dc686d17dd8b49095b3395
Author: Darin Johnson
Date: 2016-09-13T03:19:12Z
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's submitted for feedback only, please DO NOT MERGE.
It deprecates the "platform" CLI option in favor of the "launcher" option
which is the name of a class implementing the `ResponderLauncher` interface
which will invoke the run method via reflection. This allows a developer of a
different responder to merely place a jar on the classpath and specify the
appropriate `ResponderLauncher` on the classpath.
The "platform" CLI option is still made available. However, I removed the
explicit dependencies in favor of using reflection. This was done in
anticipation other refactoring the build into submodules, though this does
admittedly make the code more fragile.
> Generalize ResponderDriver to use accept a RespondLauncher class
>
>
> Key: PIRK-63
> URL: https://issues.apache.org/jira/browse/PIRK-63
> Project: PIRK
> Issue Type: Improvement
> Components: Responder
>Affects Versions: 0.1.0
>Reporter: DarinJ
>Assignee: DarinJ
>Priority: Minor
> Fix For: 0.0.1
>
>
> Currently as discussed on the mailing list, the ResponderDriver uses a switch
> statement based off a string in the CLI or properties file to determine the
> framework to launch. This is require a code change to the ResponderDriver
> for every framework added.
> The solution is to create a ResponderLauncher Interface which each framework
> overrides.
> {quote}
> public interface ResponderLauncher {
> public run()
> }
> {quote}
> To do this, we the change the string specifying the framework to the name of
> the class implementing ResponderLauncher i.e. SparkResponderLauncher and
> instantiate via reflection.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (PIRK-63) Generalize ResponderDriver to use accept a RespondLauncher class
[
https://issues.apache.org/jira/browse/PIRK-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15493275#comment-15493275
]
Tim Ellison commented on PIRK-63:
-
You can assign tasks to yourself now Darin.
> Generalize ResponderDriver to use accept a RespondLauncher class
>
>
> Key: PIRK-63
> URL: https://issues.apache.org/jira/browse/PIRK-63
> Project: PIRK
> Issue Type: Improvement
> Components: Responder
>Affects Versions: 0.1.0
>Reporter: DarinJ
>Assignee: DarinJ
>Priority: Minor
> Fix For: 0.0.1
>
>
> Currently as discussed on the mailing list, the ResponderDriver uses a switch
> statement based off a string in the CLI or properties file to determine the
> framework to launch. This is require a code change to the ResponderDriver
> for every framework added.
> The solution is to create a ResponderLauncher Interface which each framework
> overrides.
> {quote}
> public interface ResponderLauncher {
> public run()
> }
> {quote}
> To do this, we the change the string specifying the framework to the name of
> the class implementing ResponderLauncher i.e. SparkResponderLauncher and
> instantiate via reflection.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (PIRK-63) Generalize ResponderDriver to use accept a RespondLauncher class
[
https://issues.apache.org/jira/browse/PIRK-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15485963#comment-15485963
]
DarinJ commented on PIRK-63:
I can't assign the ticket to myself - but I'm working on it.
> Generalize ResponderDriver to use accept a RespondLauncher class
>
>
> Key: PIRK-63
> URL: https://issues.apache.org/jira/browse/PIRK-63
> Project: PIRK
> Issue Type: Improvement
> Components: Responder
>Affects Versions: 0.1.0
>Reporter: DarinJ
>Priority: Minor
> Fix For: 0.0.1
>
>
> Currently as discussed on the mailing list, the ResponderDriver uses a switch
> statement based off a string in the CLI or properties file to determine the
> framework to launch. This is require a code change to the ResponderDriver
> for every framework added.
> The solution is to create a ResponderLauncher Interface which each framework
> overrides.
> {quote}
> public interface ResponderLauncher {
> public run()
> }
> {quote}
> To do this, we the change the string specifying the framework to the name of
> the class implementing ResponderLauncher i.e. SparkResponderLauncher and
> instantiate via reflection.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
