Context.get raises an error if there is no event loop

[nikhilk]

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.

def get(key, default=None):
    """
    Retrieves the value stored in key from the Task.context dict. If key does not exist,
    or there is no event loop running, default will be returned
...
    """
    if not asyncio.Task.current_task():
        raise ValueError(NO_LOOP_EXCEPTION_MSG.format(key))

    return asyncio.Task.current_task().context.get(key, default)

[argaen]

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

[nikhilk]

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.

[argaen]

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