Conversation
Fixes #1048. # Conflicts: # docs/versionhistory.rst
There was a problem hiding this comment.
We should also add :raises NoEventLoopError: to various public functions in private modules. Currently, the PR has mostly only added it to public functions in public modules.
TBH, there are a lot of functions that can raise NoEventLoopError via get_async_backend(). Would it make sense to just leave off :raises NoEventLoopError:? It seems tedious to write :raises NoEventLoopError: on every public API that can call get_async_backend(). I am not aware of anyone asking for :raises ~sniffio.AsyncLibraryNotFoundError: to be explicitly documented across AnyIO's APIs in the past, anyway. I think it is implied that it can be raised from lots of AnyIO APIs, and it is self-explanatory when it happens.
Co-authored-by: Ganden Schaffner <gschaffner@pm.me>
gschaffner
left a comment
There was a problem hiding this comment.
Looks good to me. Only comment: Re:
However, I'm not so sure about sync functions. For instance, if
anyio.Eventalways used the back-end implementation you couldn't pre-set it without raising that exception. (Fortunately anyio is smarter than that.)Thus the existence of this
raises:can be a low-visual-clutter reminder that you need to be in async context to call that function/method, without the docs having to mention this explicitly everywhere it's relevant.
OK, so the consensus is that it's better to leave it out of the docs altogether?
Matthias's point regarding :raises NoEventLoopError: on sync functions1 makes sense to me. However it has some maintenance cost for little user benefit, so leaving it out entirely also seems fine to me.
Footnotes
-
I.e. a bunch of constructors,
get_cancelled_exc_class,current_time, etc. ↩
Then I suppose I'll add it back? |
I don't know about the "little benefit" … in any case, what's the alternative when you do need to discover whether e.g. you can allocate the lock for your singleton object at startup time, try it from the Python REPL and check whether that fails? |
|
I added the BTW, allocating a lock w/o an event loop has worked for a while now. It's only when you start using the methods in it that require an event loop when it looks up an binds to the actual event loop. |
Thanks. LGTM. |
|
Thanks for the reviews! |
Changes
This replaces the private
NoCurrentAsyncBackendexception with the existingNoEventLoopError.Fixes #1048.
Checklist
If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):
tests/) which would fail without your patchdocs/), in case of behavior changes or newfeatures
docs/versionhistory.rst).If this is a trivial change, like a typo fix or a code reformatting, then you can ignore
these instructions.
Updating the changelog
If there are no entries after the last release, use
**UNRELEASED**as the version.If, say, your patch fixes issue #123, the entry should look like this:
If there's no issue linked, just link to your pull request instead by updating the
changelog after you've created the PR.