bpo-32418: Add get_loop() method on Server, AbstractServer classes #4997
Conversation
|
Please add tests/NEWS entry/docs. |
Lib/asyncio/events.py
Outdated
| @@ -155,6 +155,10 @@ def close(self): | |||
| """Coroutine to wait until service is closed.""" | |||
| return NotImplemented | |||
|
|
|||
| def get_loop(self): | |||
| """ Get the event loop""" | |||
There was a problem hiding this comment.
""" Get the event loop""" -> """Get the event loop the Server object is attached to."""
| @@ -0,0 +1 @@ | |||
| Add get_loop method to Server and AbstractServer classes attached to - bpo:32418 | |||
There was a problem hiding this comment.
Drop bpo number: it will be added automatically by blurb.
| @@ -179,6 +179,9 @@ def close(self): | |||
| if self._active_count == 0: | |||
| self._wakeup() | |||
|
|
|||
| def get_loop(self): | |||
| return self._loop | |||
There was a problem hiding this comment.
Please add a test for the method.
There was a problem hiding this comment.
I cannot find tests for Server class. Could you help me please? Or it looks like i need to write test cases for Server class.
There was a problem hiding this comment.
loop.create_server returns Server objects.
| @@ -155,6 +155,10 @@ def close(self): | |||
| """Coroutine to wait until service is closed.""" | |||
| return NotImplemented | |||
|
|
|||
| def get_loop(self): | |||
| """ Get the event loop the Server object is attached to.""" | |||
| return NotImplemented | |||
There was a problem hiding this comment.
Please add a test for make sure that AbstractServer().get_loop() returns NotImplemented.
There was a problem hiding this comment.
@asvetlov It should raise NotImplementedError, please fix. NotImplemened singleton is for comparisons in Python.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Lib/test/test_asyncio/test_events.py
Outdated
| loop = asyncio.new_event_loop() | ||
| asyncio.set_event_loop(loop) | ||
| proto = MyProto(loop) | ||
| server = loop.create_server(lambda: proto, '0.0.0.0', 0) |
There was a problem hiding this comment.
The method is a coroutine, use server = loop.run_until_complete(loop.create_server(lambda: proto, '0.0.0.0', 0))
Lib/test/test_asyncio/test_events.py
Outdated
|
|
||
| def test_get_loop(self): | ||
| loop = asyncio.new_event_loop() | ||
| asyncio.set_event_loop(loop) |
There was a problem hiding this comment.
The call is not needed but loop should be closed after the test.
|
@asvetlov Done. Thanks!! Do i need to update the documentation? Looks like it. |
|
@asvetlov I have updated the docs, pls let me know what do you think? |
|
Should be documented not |
|
@asvetlov Done. |
|
Thanks! |
| proto = MyProto(loop) | ||
| server = loop.run_until_complete(loop.create_server(lambda: proto, '0.0.0.0', 0)) | ||
| self.assertEqual(server.get_loop(), loop) | ||
| loop.close() |
There was a problem hiding this comment.
This test will leak an open server object. Add server.close(); loop.run_until_complete(server.wait_closed())
|
@asvetlov There are a couple of issues with this PR, please fix if possible. |
https://bugs.python.org/issue32418