alexyuen / duke

Automatically exported from code.google.com/p/duke
0 stars 0 forks source link

Support different kinds of storage for configuration #115

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
A suggestion from phil...@helpscore.com: "It would also be desirable to allow 
the configuration to be in different formats other then on disk.  Modifying the 
Configuration class to be an interface would allow different implementation to 
choose where to pull the data from.  

I would be willing to make this change and add support for yaml configuration 
files.  It would also allow implementations to offer other forms of 
configuration such as db etc.

Please let me know if you would like me to make the changes."

Original issue reported on code.google.com by lar...@gmail.com on 20 Apr 2013 at 8:21

GoogleCodeExporter commented 8 years ago
I think supporting different kinds of storage of the configuration is a 
worthwhile improvement, so if you would do that it's great.

However, I'm wondering if changing Configuration into an interface is the right 
thing to do. Probably we'll just have a single implementation that gets 
populated in different ways. The configurations are going to be so small that 
always reading the entire thing into memory for use is no problem at all.

What would be nice, though, is having an interface ConfigurationReader that can 
return a Configuration object that's fully populated. That way we can more 
easily write code that loads configurations in different ways. 

What do you think?

Anyway, if you want to work on this that would be great. You can make a clone 
and push your changes into it, then I can pull it into the main repository from 
there. Also, you'll be credited in Mercurial and on Ohloh for the commits: 
https://www.ohloh.net/p/duke-dedup/contributors/summary

Original comment by lar...@gmail.com on 20 Apr 2013 at 8:31

GoogleCodeExporter commented 8 years ago
I could see a reader as another approach but I was thinking of the 
configuration being the interface for 2 reasons.  

First, it would allow the use of a common shared configuration when integrating 
Duke into a larger application.  The reader would address this. 

Second, by having configuration be an interface it would allow configuration to 
have the ability to reload data without restarting the application. I suppose 
it would be possible to work around this by deriving from Configuration.

Original comment by phil...@helpscore.com on 23 Apr 2013 at 9:02

GoogleCodeExporter commented 8 years ago
I make a change in my clone project 
philipw-duke-head which addresses this issue.  Please check it out and let me 
know if I need to change anything.  I went with the interface approach since 
the ConfigLoader already offers the ability to load a standard configuration.

Original comment by phil...@helpscore.com on 25 Apr 2013 at 4:56

GoogleCodeExporter commented 8 years ago
This is good stuff. I pushed it into the mainline now. Thanks! :-)

Original comment by lar...@gmail.com on 25 Apr 2013 at 6:09

GoogleCodeExporter commented 8 years ago
I'm not too happy with the Configuration -> ConfigurationInterface name change. 
The name we keep referring to in the code is that of the interface, so I'd 
really like that to be Configuration. That way, client code should need far 
less changes, too.

So unless you protest, I'm going to change ConfigurationInterface -> 
Configuration, and Configuration -> ConfigurationImpl.

Original comment by lar...@gmail.com on 25 Apr 2013 at 7:07

GoogleCodeExporter commented 8 years ago
I was originally going to do the Configuration and ConfigurationImpl.  And I 
too was not happy with implementing in that fashion.  The only reason I broken 
with the standard was because it would impact all of the current 
implementations that are out in the wild.  So I went with the non-standard 
naming. 

I am actually happy that you changed it to be more standard.

Phil

Original comment by phil...@helpscore.com on 29 Apr 2013 at 7:29

GoogleCodeExporter commented 8 years ago
Glad you agree. :-) I've made the change now, so you can just do a pull and 
carry on.

Original comment by lar...@gmail.com on 4 May 2013 at 8:31