Skip to content

Add missing methods to asyncio stubs #3088

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Aug 6, 2019
Merged

Conversation

cole
Copy link
Contributor

@cole cole commented Jun 26, 2019

Addresses a number of missing methods described in #2313.

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, just one remark.

In general, all functions marked @coroutine should actually use async def, instead, at least for added definitions.

@cole
Copy link
Contributor Author

cole commented Jul 2, 2019

@srittau ah, of course. I've made that change, but it seems to break pytype. I feel like I'm missing something obvious..?

I've also added a few more items noted in #2313 — let me know if any of those changes need revisions.

@srittau
Copy link
Collaborator

srittau commented Jul 2, 2019

@rchen152 Does pytype not support async def yet?

@rchen152
Copy link
Collaborator

rchen152 commented Jul 2, 2019

Huh, I'm surprised this hasn't come up before. pytype only supports the @coroutine decorator in stubs, not async def. I'll go ahead and add support for async def and update here when it's released. (If it turns out to be easy, it'll probably be out tomorrow; otherwise, it'll be delayed to next week, since the rest of this week is a holiday for me.)

@rchen152
Copy link
Collaborator

rchen152 commented Jul 9, 2019

This is turning out to be more troublesome than I anticipated =(
I've prepared a change making async a keyword in stubs, but its rollout is being held up by Google code that uses async as a variable name. It's hard to say how long it'll take to deal with - anywhere from a few days to a few weeks depending on how many occurrences I find and how responsive code owners are.

In the meantime, if you'd like to get this PR in without waiting on pytype, maybe async def foo() -> X could be rewritten to def foo() -> Coroutine[Any, Any, X]?

@srittau
Copy link
Collaborator

srittau commented Jul 9, 2019

Let's just use @coroutine for now, like the PR did originally.

@cole
Copy link
Contributor Author

cole commented Jul 10, 2019

Ready for re-review.

rchen152 added a commit to google/pytype that referenced this pull request Jul 11, 2019
People are starting to want to use this syntax in typeshed:
python/typeshed#3088.

One odd thing I found is that we were allowing `async.coroutine` as a
decorator, which doesn't work once we make async a keyword, but I think the
decorator is supposed to be async*io*.coroutine anyway.

PiperOrigin-RevId: 257443556
@rchen152
Copy link
Collaborator

FYI with #3112, pytype should now support the async def syntax. I guess I shouldn't have been so pessimistic about how long it would take to get that change released ^^;

cole added 14 commits July 11, 2019 19:32
The sock_* methods used to return futures, and are now async methods.
As noted in the docs:

    Even though this method was always documented as a coroutine
    method, releases before Python 3.7 returned a Future. Since Python
    3.7 this is an async def method.
Added start_serving to create_server and create_unix_server.
Added ssl_handshake_timeout to create_server, create_unix_server,
create_connection, create_unix_connection, and connect_accepted_socket.
@cole cole force-pushed the asycio-37-stubs branch from 0b68e15 to 2fcf336 Compare July 12, 2019 03:11
@cole
Copy link
Contributor Author

cole commented Jul 12, 2019

Reverted to async def 😄

@cole
Copy link
Contributor Author

cole commented Aug 5, 2019

@srittau are there any changes I should make to this PR?

@srittau srittau merged commit 29771aa into python:master Aug 6, 2019
@srittau
Copy link
Collaborator

srittau commented Aug 6, 2019

Sorry, I missed this completely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants