magfest / ubersystem

MAGFest's Ubersystem - handles ticketing, staffing, analytics, volunteers, and tons more
http://magfest.org
GNU Affero General Public License v3.0
48 stars 55 forks source link

BUG: config files won't interpolate values from outside of their section #189

Open binary1230 opened 10 years ago

binary1230 commented 10 years ago

_unrepr() in uber/config.py pukes on the following code in production.conf:

[cherrypy]
server.socket_host = "0.0.0.0"
server.socket_port = 4321

(it's something to do with the [cherrypy] header)

workaround is to add path = "/magfest" in there like so:

[cherrypy]
path = "/magfest"
server.socket_host = "0.0.0.0"
server.socket_port = 4321
binary1230 commented 10 years ago

When this is fixed, update README.md to remove the workaround

EliAndrewC commented 10 years ago

Tagging this with "before prereg launches".

binary1230 commented 10 years ago

I accidentally ended up digging into this more.

The reason is the following:

configspec.ini says:

path = string(default="/magfest")
...
[cherrypy]
...
tools.sessions.path = string(default="%(path)s")

That attempts to reference path which is defined in the section outside of [cherrypy]

However, it appears either because of the way we're reading the config files, or because of a bug in ConfigObj, that you can't do string interpolation (i.e. using %(path)s) for variables like path which exist outside your section.

It appears to be related to using multiple config files and how we're combinig them. I'm working around the issue by copying values around in the config file, like TEMPLATES_DIR, for now.

binary1230 commented 10 years ago

It might be related to this: https://code.google.com/p/configobj/issues/detail?id=18

But that might also be a red herring.

This documentation here: http://www.voidspace.org.uk/python/configobj.html#string-interpolation

Seems like it says it should be doing what we want:

Interpolation checks first the current section to see if name is the key to a value. ('name' is case sensitive).

If it doesn't find it, next it checks the 'DEFAULT' sub-section of the current section.

If it still doesn't find it, it moves on to check the parent section and the parent section's 'DEFAULT' subsection, and so on all the way up to the main section.

If the value specified isn't found in any of these locations, then a MissingInterpolationOption error is raised (a subclass of ConfigObjError).
EliAndrewC commented 10 years ago

Fortunately, I'm one of the maintainers of configobj so I have commit access to that repo for whenever I manage to get around to making the fix :)

binary1230 commented 10 years ago

Also, this probably isn't related to _unrepr() at all, that's just a red herring. It's caused by anything accessing the underlying data.

So in theory if you just do the following it should reproduce the issue: print(conf['cherrypy'])

binary1230 commented 10 years ago

Eli, goddamnit. I love how I keep discovering that you're the maintainer of all our external libs :+1: lolz.

binary1230 commented 10 years ago

Last bit of info.

If you type in print(conf['cherrypy']) and step in there, all the data is there.

But when it goes down to find the interpolated key, the parent section appears not to have the merged data from our config files, it only has the FIRST config file. I think. That's where we should pick up the chase.

EliAndrewC commented 10 years ago

I once submitted a patch to cherrypy which had 480 unit tests, and they ignored it. Now that they're on bitbucket instead of subversion, I can actually make a pull request instead of just submitting a patch like people had to do back in the dark ages, so I should probably try again :)

EliAndrewC commented 10 years ago

Re-tagging this as low priority because it no longer affects the Uber config file. We should still figure this out, but we no longer need the annoying workaround.