airbrake-python
airbrake-python copied to clipboard
airbrake
Improve init signature
There are a number of options to improve on the current init signature. Right now it's a list of parameters with defaults.
What's wrong with the current situation?
- A bunch of arguments with defaults is a bit brittle. We have to change the signature every time we add new options.
- In https://github.com/airbrake/airbrake-python/pull/53 I added **kwargs for variable arguments at the end. Configuration values are in named arguments with defaults, anything setting contexts ends up in the variable arguments.
The only real alternative I can think of is to pass the context by ref in one arg and create it separately from the Airbrake object.
The only real alternative I can think of is to pass the context by ref in one arg and create it separately from the Airbrake object.
This sounds like a fine idea to me.
The only thing holding me back here is looking at the other approaches from rollbar and sentry. Looks like they both use **kwargs, https://github.com/rollbar/pyrollbar/blob/master/rollbar/init.py#L263 and https://github.com/getsentry/raven-python/blob/master/raven/base.py.
I'm starting to think we should stick with **kwargs, but in the readme suggest that users create a separate dict object with all the config, then pass that in. I think it's nice to be able to init the library in one line, and forcing config to another structure might be a tad inconvenient.
This might be a question of what you expect from your SDK. If I pass in a context key/value pair that airbrake doesn't support, should I get an error when instantiating the notifier class? Or will it bomb out (possibly in a background thread after https://github.com/airbrake/airbrake-python/issues/50) when the airbrake api returns a non-2xx ?
If the SDK code should keep track of and check the context schema at initialization time, then there ought to be updates to the code here if the airbrake api changes or supports new (or when it deprecates) the values supported in context.
FWIW, my preference is when the SDK is stupid-proof when possible, even if that means a little bit of overhead, both for the maintainers of the library or the clients using airbrake.
I am not super opinionated, I just want to help out if I can. I still think it is wise to avoid a grab-bag variable.
In python3, a user could certainly build their own dictionaries, and pass them in using ** multiple times, like Airbrake(..., **context, **config) which is nice.
We have to change the signature every time we add new options
It's fair to want to avoid this for the Airbrake class, but perhaps there could be a utility/helper function that will help you build a context dictionary with some useful defaults.
If I pass in a context key/value pair that airbrake doesn't support, should I get an error when instantiating the notifier class?
Ya, I think errors at init time are ideal here, we should add a check for this.
If the SDK code should keep track of and check the context schema at initialization time, then there ought to be updates to the code here if the airbrake api changes or supports new (or when it deprecates) the values supported in context.
Yup, agreed.
It's fair to want to avoid this for the Airbrake class, but perhaps there could be a utility/helper function that will help you build a context dictionary with some useful defaults.
I think that's a great idea. So ideally this function would also:
- Include context checking, and raise exceptions when unsupported options are passed in.
- Be reused internally to check context dicts that users pass in.