Modify config #30
Modify config #30
Conversation
This reworks how configuration works so instead of it being global, it's now bound to a Falcon API instance through get_app() and then passed to the resources. This will make it easier to have instance-specific configuration where two instances of the same class can have different configuration differentiated by namespace.
This lets us import classes from a spec string.
|
@peterbe Can you do a quick r? for python-y issues and a sanity check? |
I accidentally unindented this. Fixing it.
| try: | ||
| return getattr(module, class_name) | ||
| except AttributeError: | ||
| raise ValueError('%s is not a valid member of %s' % ( |
There was a problem hiding this comment.
Why a ValueError? What was wrong with the AttributeError? Also, might it be of interest to re-wrap it as ImportError?
There was a problem hiding this comment.
We could return the AttributeError, my initial tinkering suggested it wasn't wildly helpful and that a better error messages was. Without this wrapping, you get something like:
>>> import importlib
>>> module = importlib.import_module('hashlib')
>>> getattr(hashlib, 'foo')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: 'module' object has no attribute 'foo'
>>>
"module" is totally not helpful here. With my changes, you get something like:
ValueError: 'foo' is not a valid member of hashlib'
I thought about wrapping the ImportError and then decided not to because the ImportError itself is more informative than what I'd probably do if wrapping it.
There was a problem hiding this comment.
Continuing, I think the actual exception is moot. This is something that the developer is seeing when they're implementing something and a user is seeing if they've got antenna configured wrong. It's not an exception that is intended to be caught and handled.
I'm on the fence. I think I want to hedge towards users being able to see the message and intuit what to do next. I don't know what to do to make it better.
With Python 3, this is much easier since you can chain exceptions. Then we could throw an additional ConfigurationError with a super helpful message and the user gets our message plus the original one.
|
r+ |
|
Thank you! |
This tweaks configuration a little bit to make it easier to work with in the breakpad code I'm writing.
The only new feature is the
parse_classparser. That should be pretty straight-forward.The rest of it is nixing the config singleton for one that's instantiated when the app is created.