Config refactoring proposal

Dmitri Dolguikh dmitri at redhat.com
Tue Feb 23 21:24:00 UTC 2010


pls. see answers inline.

On 02/23/2010 04:36 PM, Jesus M. Rodriguez wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> While working on Pinsetter component of Candlepin, I have the need to
> configure both Quartz (via org.quartz.* properties) and Pinsetter itself
> (i.e. needing a list of task classnames as well as their cron schedules).
> I went through the config code to see if I could get a better
> understanding of what is going on with the application and wondering
> why we had so many "Configuration" classes. So to relieve some of my
> confusion I'm proposing the changes listed below. Thoughts?
>
> 1) Rename JerseyGuiceConfiguration to CandlepinContextListener -
>     it will go along way to remove my confusion as this is the basic
>     entry point into the guice configuration. I understand the lifecycle
>     of the ContextListener over JerseyGuiceConfiguration which 
> confuses me.
sure.

> 2) Rename CandlepinProductionConfiguration to CandlepinModule -
>     this is a guice module for candlepin. Again the current name
>     confuses me.
no objections here either.

> 3) Make Config class injectable into other parts of the application
>     so that we may get access to configuration data when needed. Plus
>     there should only be one Config, right now I see two instances where
>     we created a "new Config()" class.
internally it behaves as a singleton. i tend to avoid static methods, as 
they are impossible to test (or make components that depend on them 
untestable). while current appearance maybe deceiving in regards to its 
internals, i don't think it's a major point of confusion. in short, i 
think it's better to keep the class free of static members.

> 4) Roll the ConfigFileLoader into the Config class, it's simple enough
>     and Config basically comes from a file if it exists, so I think
>     it should live in there.
more or less the same reasoning (testable is good). single 
responsibility makes classes easier to test (and understand).

> 5) JPAConfiguration seems pretty generic and could be rolled into Config
>     as well.  As part of this make CandlepinModule call
>     config.getNamespaceProperties("jpa.config") instead of new
>     Config().jpaConfiguration()
at this point you already know my opinion about small classes. i like 
them :). i would strongly suggest not to use strings in actual code to 
retrieve bits of configuration. actual methods are easier to 
locate/maintain. at this point (pls. correct me if i'm wrong) we have a 
limited number of namespaces, which makes the approach (methods vs. 
string constants) tractable (& preferable).

> 6) Move JPAInitializer into CandlepinModule
sure.

> 7) I'm still working on undestanding CustomizableModules
this allows to define what external modules should be loaded (via 
candlepin.conf file). needed for implementations of extension points by 
third parties (such as IT).

> 8) add some utility methods to Config i.e. getString(), getInt() etc
>     (maybe when I needed them)
sure?

-dmitri
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.fedorahosted.org/pipermail/candlepin/attachments/20100223/21902c07/attachment.html>


More information about the candlepin mailing list