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 <dar...@apache.org>
> 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 infrastruct...@apache.org or file a JIRA ticket
> with INFRA.
> ---
> 

Reply via email to