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 responders available? So, for example, having the driver maintain a list of responders by name, and letting people specify the name on the command line. Each responder would then be responsible for implementing a standardised interface, and registering themselves with the driver by name. In that model the responders would each know about (a) the driver, and how to register themselves by name, and (b) implement a standard life-cycle for building a response. The driver would be responsible for (a) collecting and maintaining the registrations of any responder being loaded, and (b) invoking the correct responder based on user selection. Make sense? I can hack something together to show what I mean. Regards, Tim On 19/09/16 07:05, DarinJ wrote: > 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 <[email protected]> > 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. > > ---- > > > --- > 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 is enabled but not working, please > contact infrastructure at [email protected] or file a JIRA ticket > with INFRA. > --- >
