[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)