elanbeat / gcfg

Fork of https://code.google.com/p/gcfg/. Automatically exported by GoogleCodeExporter.
Other
0 stars 0 forks source link

config files with 'extra' content #4

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
good work by the way :)

I've got a project with a config file shared amongst several utilities, and 
some test/work-in-progress settings. I'd really prefer it if the ReadInto 
didn't fail completely if it finds a section in the file that doesn't match a 
struct in the receiver.
I cloned the repo and:
- changed set.go line 30 to return nil
- reversed the test result requirement of line 137 in test_read.go

I'm not sure if I'll have to make further changes to ignore missing subsections 
(I'm not using them yet)... my tests pass now :)

I don't know if this fits with your patterns, but now it just ignores any 
entries in the config file that don't match a struct (silently at the moment, 
but a stderr warning might be good)

It's not an 'issue' as such, so I won't be offended if you ignore this, keep up 
the outstanding work!

Original issue reported on code.google.com by MFHHol...@gmail.com on 24 Oct 2013 at 10:47

GoogleCodeExporter commented 9 years ago
Thank you for the feedback! It is a valid request, but at the same time I do 
have some concerns about it.

It is not stated explicitly in the package docs, but the thinking behind 
disallowing "unexpected" data (variable, section, or subsection) is similar to 
that for unused variables in Go: it can indicate configuration mistakes, e.g. 
using mismatched versions of a utility and its config file.

> I've got a project with a config file shared amongst several utilities...
Depending on the level of cohesion among the utilities, I'd consider sharing 
the config struct, or splitting configuration into shared and utility-specific 
files. Would one of those not be suitable in your case?

> ...and some test/work-in-progress settings.
To be honest, I don't find "experimenting" a very convincing argument for 
ignoring extra data. (The rationale is the same as above.) In addition, just 
allowing extra (sub)sections (but not extra variables in "known" sections) also 
would seem somewhat arbitrary.

One approach could be to continue parsing on extra data and return a separate 
kind of error, so that the caller can decide to ignore it. Another one (which 
I've been trying to avoid) is to add some configuration for gcfg itself. But 
I'm not convinced if either is worth the added complexity.

Original comment by speter....@gmail.com on 2 Nov 2013 at 1:53

GoogleCodeExporter commented 9 years ago
I like your thinking; it's a completely valid point.

I'll go back and have a look at the project and see if I can go about it a 
different way that fits better with Go methodology.

Original comment by MFHHol...@gmail.com on 2 Nov 2013 at 3:30

GoogleCodeExporter commented 9 years ago
One use case strikes me: upgrades.
If I add a config field to the struct, then any existing config files out there 
in existing installations will immediately be invalid and will refuse to load. 
In order to update them I have to load the data from the existing file and add 
any missing fields, but I can't do this using the gcfg library (which would 
seem to make sense).

Maybe creating an upgrade utility that 'knows about' previous config file 
versions and loads the config file using an older struct and then modifies it 
to the latest version, but that seems like a lot of work for a simple problem?

Original comment by MFHHol...@gmail.com on 2 Nov 2013 at 3:47

GoogleCodeExporter commented 9 years ago
Upgrade should not give an error unless you remove some sections / variables in 
the config struct. (I.e., missing values for declared variables do not cause an 
error, but values provided for undeclared variables do. Sorry, maybe my 
comparison with unused variables in Go was confusing -- the rationale is 
equivalent but "unused" applies in a different way.)

Updating gcfg files programmatically is currently not supported. So in case 
supporting upgrades (with new config fields) without change of configuration 
file is needed, the application must make sure that it will work even if the 
user doesn't provide values for the new fields. (E.g. handling zero-values 
accordingly, or pre-populating sensible defaults etc.)

Downgrades can cause an error if new variables have been added to both the 
config struct and the config file, and then only the application is downgraded. 
I tend to think this behavior is better than silently ignoring the values that 
are not supported by the prior version.

Original comment by speter....@gmail.com on 2 Nov 2013 at 4:27