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

add a new is_readable method to SocketType (fix #760) #1137

Merged
merged 2 commits into from
Jul 6, 2019
Merged

Conversation

ziirish
Copy link
Contributor

@ziirish ziirish commented Jul 3, 2019

As explained in #760, this PR adds a new is_readable method to trio.socket.SocketType object.

Copy link
Member

@njsmith njsmith left a comment

Choose a reason for hiding this comment

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

Looks good overall, with a few minor comments:

@@ -404,6 +404,23 @@ class MySocket(stdlib_socket.socket):
assert await client.recv(1) == b"x"


async def test_SocketTupe_is_readable():
Copy link
Member

Choose a reason for hiding this comment

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

Typo: "SocketTupe" should be "SocketType"

assert not client.is_readable()
await server.send(b"x")
assert client.is_readable()
assert await client.recv(1) == b"x"
Copy link
Member

Choose a reason for hiding this comment

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

This can be simplified a lot by using socketpair, which directly returns a pair of connected sockets. Something like:

a, b = trio.socket.socketpair()
with a, b:
    assert not a.is_readable()
    await b.send(b"x")
    await _core.wait_readable(a)
    assert a.is_readable()
    assert await a.recv(1) == b"x"
    assert not a.is_readable()

I also added a call to wait_readable, just in case the operating system needs a moment to notice that writing to b makes a readable.

@@ -0,0 +1,3 @@
We added a new ``is_readable`` method to the ``trio.socket.SocketType``
object that allows you to check whether a socket is ready to be read
or not.
Copy link
Member

Choose a reason for hiding this comment

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

The double-backticks are raw code; since we're referring to objects that have entries in the documentation, we can use single-backticks to link directly:

Trio sockets have a new method `~trio.socket.SocketType.is_readable`, that allows you
to check whether a socket is readable. This is useful for HTTP/1.1 clients.

(The tilde is a shorthand that means "make a link to trio.socket.SocketType.is_readable, but just render it as is_readable".)

@codecov
Copy link

codecov bot commented Jul 5, 2019

Codecov Report

Merging #1137 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1137      +/-   ##
==========================================
+ Coverage   99.51%   99.51%   +<.01%     
==========================================
  Files         104      104              
  Lines       12636    12659      +23     
  Branches      969      970       +1     
==========================================
+ Hits        12575    12598      +23     
  Misses         40       40              
  Partials       21       21
Impacted Files Coverage Δ
trio/_socket.py 99.58% <100%> (+0.01%) ⬆️
trio/tests/test_socket.py 100% <100%> (ø) ⬆️

1 similar comment
@codecov
Copy link

codecov bot commented Jul 5, 2019

Codecov Report

Merging #1137 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1137      +/-   ##
==========================================
+ Coverage   99.51%   99.51%   +<.01%     
==========================================
  Files         104      104              
  Lines       12636    12659      +23     
  Branches      969      970       +1     
==========================================
+ Hits        12575    12598      +23     
  Misses         40       40              
  Partials       21       21
Impacted Files Coverage Δ
trio/_socket.py 99.58% <100%> (+0.01%) ⬆️
trio/tests/test_socket.py 100% <100%> (ø) ⬆️

@ziirish
Copy link
Contributor Author

ziirish commented Jul 5, 2019

Thanks for the review.
I have addressed the comments.

@njsmith njsmith merged commit 254ded3 into master Jul 6, 2019
@njsmith njsmith deleted the gh-760 branch July 6, 2019 02:44
@njsmith
Copy link
Member

njsmith commented Jul 6, 2019

Thanks!

@ghost
Copy link

ghost commented Dec 16, 2021

How do I use the is_readable attribute? I get ("'SocketStream' object has no attribute 'is_readable'",) Any way to load it or something?

@oremanj
Copy link
Member

oremanj commented Dec 16, 2021

This is a method of the underlying socket object, not the SocketStream. Use stream.socket.is_readable(), not stream.is_readable().

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