Cargo

Discover implementations at runtime

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 0.9
  • Fix Version/s: 1.0
  • Component/s: Core
  • Labels:
    None
  • Complexity:
    Intermediate
  • Number of attachments :
    2

Description

As it stands right now, Cargo DefaultXXXFactory only knows about implementations that are hard-coded in them. This makes it unnecessarily hard for people (like me) to develop container implementations. I can't provide the same level of usability to users unless I get into the core.

But this needs not be the case. We should use the standard META-INF/services look up to discover implementations at runtime.

In this way, there's an easier path for additional container implementations — they can start on their own outside Cargo, be proven usable, and then gradually get into the Cargo project itself if that makes sense. If for whatever reasons it needs to stay outside the main Cargo project, that can work, too.

I'm willing to provide a patch for this change if the developers are OK with the direction.

  1. CARGO-584.diff
    05/Aug/08 11:33 AM
    31 kB
    Kohsuke Kawaguchi
  2. CARGO-584.diff
    22/Jul/08 12:40 PM
    13 kB
    Kohsuke Kawaguchi

Activity

Hide
Matt Wringe added a comment -

cargo fully supports custom container implementations without having to make any changes to cargo itself, all you have to do is specify the implementation class to use:

<plugin>
 <groupId>org.codehaus.cargo</groupId>
 <artifactId>cargo-maven2-plugin</artifactId>
 <configuration>
   <container>
     <containerId>foo</containerId>        
     <implementation>com.foo.Container</implementation>
     ...
    </container>
     <configuration>
       <implementation>com.foo.Configuration</implementation>
       ...
     </configuration>
     <deployer>
        <implementation>com.foo.Deployer</implementation>
         ...
     </deployer>
 </configuration>
</plugin>

Discussing the rest in emails.

Show
Matt Wringe added a comment - cargo fully supports custom container implementations without having to make any changes to cargo itself, all you have to do is specify the implementation class to use:
<plugin>
 <groupId>org.codehaus.cargo</groupId>
 <artifactId>cargo-maven2-plugin</artifactId>
 <configuration>
   <container>
     <containerId>foo</containerId>        
     <implementation>com.foo.Container</implementation>
     ...
    </container>
     <configuration>
       <implementation>com.foo.Configuration</implementation>
       ...
     </configuration>
     <deployer>
        <implementation>com.foo.Deployer</implementation>
         ...
     </deployer>
 </configuration>
</plugin>
Discussing the rest in emails.
Hide
Kohsuke Kawaguchi added a comment -

This is an useful feature, but there are several issues with this.

First, this only works when you use cargo-maven2-plugin, but the maven plugin is just one of the offerings.

Second, it violates the DRY principle. When the user added additional container implementation through <dependency> in the plugin configuration, that should be a signal enough for Cargo. The users shouldn't be asked to put lengthy information like this.

The point of factory is so that users won't need to worry about the actual implementation class names. If they need to hard-code it, they must just as well call the new FooContainer() themselves.

Show
Kohsuke Kawaguchi added a comment - This is an useful feature, but there are several issues with this. First, this only works when you use cargo-maven2-plugin, but the maven plugin is just one of the offerings. Second, it violates the DRY principle. When the user added additional container implementation through <dependency> in the plugin configuration, that should be a signal enough for Cargo. The users shouldn't be asked to put lengthy information like this. The point of factory is so that users won't need to worry about the actual implementation class names. If they need to hard-code it, they must just as well call the new FooContainer() themselves.
Hide
Matt Wringe added a comment -

please discuss this on the mailing lists

Show
Matt Wringe added a comment - please discuss this on the mailing lists
Hide
Kohsuke Kawaguchi added a comment -

Pointer to the discussion in the ML for others to follow: http://www.nabble.com/Implementation-pluggability-td18330148.html

Show
Kohsuke Kawaguchi added a comment - Pointer to the discussion in the ML for others to follow: http://www.nabble.com/Implementation-pluggability-td18330148.html
Hide
Vincent Massol added a comment -

Definitely +1 from me for this as this is something I had wanted to do since the beginning but had always left for later.

Show
Vincent Massol added a comment - Definitely +1 from me for this as this is something I had wanted to do since the beginning but had always left for later.
Hide
Kohsuke Kawaguchi added a comment -

Patch that shows the direction of changes.
Sanity checking before I proceed on doing the same for all the other factories.

Show
Kohsuke Kawaguchi added a comment - Patch that shows the direction of changes. Sanity checking before I proceed on doing the same for all the other factories.
Hide
Kohsuke Kawaguchi added a comment -

Applied the same technique to all the factory types.

Show
Kohsuke Kawaguchi added a comment - Applied the same technique to all the factory types.
Hide
Kohsuke Kawaguchi added a comment -

Committed as rev.1681.

Show
Kohsuke Kawaguchi added a comment - Committed as rev.1681.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: