Open nikhilk opened 6 years ago
The behavior you mention was an old one (and apparently forgot to update the docs). I did this opinionated change to make it more explicit that there was no loop, otherwise you can run into situations where you think you are retrieving/setting some variable when you aren't.
I know it forces you to use more lines in your code because you have to do:
try:
bar = context.get('foo')
except ValueError:
bar = None
but I prefer this part being explicit
I am not sure ValueError communicates that there isn't a loop. The doc on ValueError says this: "Raised when an operation or function receives an argument that has the right type but an inappropriate value"
Furthermore, the doc still in current sources indicates, default will be returned if there is no loop. I think explicitly specifying default has the same effect in making it more explicit in the code... if you think its insufficient, you could add an optional flag require_loop, and default it to True. Then someone could just do this:
context.get(key, default=None, require_loop=False)
and thats pretty explicit and self-documenting as well.
Agreed ValueError
maybe is not the best exception to raise, RuntimeError
maybe is more adequate
I think explicitly specifying default has the same effect in making it more explicit in the code
Nope, with the returning a default
strategy you can fall into having defaults
all over your code without you noticing. Same for set
operation, where you may think you are setting it but you're not.
If you don't like the try
catch
code you can go with suppress
:
value = default
with contextlib.suppress(ValueError):
value = context.get('key')
I know its not as easy as having a param in the function call, but I really want users of this to think about what they are doing, we suffered this in production because of treating the same way not being properly initialized or not having the key in the context.
Furthermore, the doc still in current sources indicates, default will be returned if there is no loop
Will update, thanks for reporting
https://github.com/Skyscanner/aiotask-context/blob/master/aiotask_context/__init__.py#L89
The doc string says that if there is no event loop, the default value will be returned.
The code however raises a ValueError.
Logging this bug, as I think the documented behavior is desired, not the current implementation.