Skip to content
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

Fix examples/docs for websocket send commands being a coroutine #1886

Merged
merged 1 commit into from
May 13, 2017
Merged

Fix examples/docs for websocket send commands being a coroutine #1886

merged 1 commit into from
May 13, 2017

Conversation

balloob
Copy link
Contributor

@balloob balloob commented May 13, 2017

What do these changes do?

Version 1.3 made websocket send commands awaitable: b3c80ee. However that change did not update tests/examples. This PR finishes that work.

It went unnoticed because when a send method is called, the bytes are actually written to the socket and a coroutine is returned for the drain command.

Noticed it because setting PYTHONASYNCIODEBUG to 1 started printing errors about unawaited coroutines.

Are there changes in behavior for the user?

No

Related issue number

No

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt
    • The format is <Name> <Surname>.
    • Please keep alphabetical order, the file is sorted by names.
  • Add a new entry to CHANGES.rst
    • Choose any open position to avoid merge conflicts with other PRs.
    • Add a link to the issue you are fixing (if any) using #issue_number format at the end of changelog message. Use Pull Request number if there are no issues for PR or PR covers the issue only partially.

@balloob balloob changed the title Fix websocket send commands being a coroutine Fix examples/docs for websocket send commands being a coroutine May 13, 2017
@codecov-io
Copy link

codecov-io commented May 13, 2017

Codecov Report

Merging #1886 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1886   +/-   ##
=======================================
  Coverage   97.06%   97.06%           
=======================================
  Files          37       37           
  Lines        7556     7556           
  Branches     1314     1314           
=======================================
  Hits         7334     7334           
  Misses        100      100           
  Partials      122      122
Impacted Files Coverage Δ
tests/autobahn/client.py 97.09% <100%> (ø) ⬆️
examples/web_ws.py 95.18% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update beb1262...7aa27a9. Read the comment docs.

@asvetlov asvetlov merged commit 127c451 into aio-libs:master May 13, 2017
@asvetlov
Copy link
Member

Thanks!

@balloob balloob deleted the fix-ws-tests branch May 15, 2017 01:26
@balloob
Copy link
Contributor Author

balloob commented May 15, 2017

When I started yielding from sending messages I quickly ran into Python bug 29930: 2 drains in parallel will raise an AssertionError error.

Do you think that we should add this to the aiohttp docs?

@AraHaan
Copy link
Contributor

AraHaan commented May 15, 2017

Yeah, this is exactly why I use the latest commit of 3.6.1+ by building python from source every day for 32 and 64 bit windows. Also all the more reason to use 'async def' and 'await' as well. I also can't wait for the day when python 3.4.x reaches no further patches by EOL so that way aiohttp can drop python 3.4 and go full 'async def' and 'await'.

@asvetlov
Copy link
Member

I think the main showstopper for dropping Python 3.4 support is absence of 3.5 in Debian stable.

@AraHaan
Copy link
Contributor

AraHaan commented May 15, 2017

Yeah, if I was part of Debian development I would change that to latest commit of 3.6.1+.

After all I do not think 3.6.1+ from github would not break most things for 3.4. But it would allow people to use 'async def' and 'await' on the fly without needing for them to manually compile 3.5 or 3.6 to begin with.

@asvetlov asvetlov mentioned this pull request May 24, 2017
@thehesiod
Copy link
Contributor

@asvetlov stretch shipped, IIRC it's 3.5

@asvetlov
Copy link
Member

asvetlov commented Jul 3, 2017

Yes, I know.
We should give people several months for migrating on Debian Stretch.
Maybe Autumn is good time for shipping aiohttp 3.0 with dropped Python 3.4 support.

@balloob do you know plans for Raspberry PI?

@balloob
Copy link
Contributor Author

balloob commented Jul 5, 2017

Raspberry PI is expecting a Debian Stretch based release over the summer. Last time it was ~5 months after the Debian stable release.

(source - last paragraph "One final thing")

@lock
Copy link

lock bot commented Oct 28, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants