| msg336287 - (view) |
Author: Kevin Mai-Hsuan Chia (mhchia) * |
Date: 2019-02-22 09:14 |
| It seems the result of `asyncio.Server.sockets` after `asyncio.Server.close()` is performed becomes `[]` instead of `None` since python 3.7. However, in the [document 3.7 and 3.8](https://docs.python.org/3.8/library/asyncio-eventloop.html#asyncio.Server.sockets), it states ``` List of socket.socket objects the server is listening on, or None if the server is closed. Changed in version 3.7: Prior to Python 3.7 Server.sockets used to return an internal list of server sockets directly. In 3.7 a copy of that list is returned. ``` For me, I think the comment `Changed in version 3.7: ...` only emphasizes the "copied list" is returned. IMO it will be more clear if the change from `None` to `[]` is mentioned in the comment as well. Sorry if this issue is not appropriate. Thanks! |
|
|
| msg336299 - (view) |
Author: Emmanuel Arias (eamanu) * |
Date: 2019-02-22 12:31 |
| Yes, that is a problem (in principle) of the docs. But I think that would be appropriate return None if is closed. |
|
|
| msg336301 - (view) |
Author: Emmanuel Arias (eamanu) * |
Date: 2019-02-22 12:46 |
| hmm looking in depth when the server is closed, currently return None. |
|
|
| msg336302 - (view) |
Author: Emmanuel Arias (eamanu) * |
Date: 2019-02-22 12:48 |
| Maybe would be great mention that the None is still return for closed server |
|
|
| msg336304 - (view) |
Author: Kevin Mai-Hsuan Chia (mhchia) * |
Date: 2019-02-22 13:20 |
| Excuse me, I tried 3.7.2 and 3.8.0a1+ with the following code and still failed. Could you help me to try this? or can you give me a pointer to the code returning `None`? Sorry for the inconvenience and thanks a lot. ``` import asyncio import platform async def close_server(): def handler(reader, writer): pass s = await asyncio.start_server(handler, host='127.0.0.1', port=34567) s.close() await s.wait_closed() assert s.sockets is None print("version: ", platform.python_version()) asyncio.get_event_loop().run_until_complete(close_server()) ``` |
|
|
| msg336305 - (view) |
Author: Emmanuel Arias (eamanu) * |
Date: 2019-02-22 13:28 |
| @mhchia yes! sorry! I was testing the .close() return. yes you are right is confused. I will modified the PR. Thanks |
|
|
| msg336311 - (view) |
Author: Kevin Mai-Hsuan Chia (mhchia) * |
Date: 2019-02-22 13:53 |
| Sure thing. Should it be discussed more detailed before the PR? If the document or the returned value is controversial, I would like to contribute if needed. Thanks :) |
|
|
| msg336312 - (view) |
Author: Emmanuel Arias (eamanu) * |
Date: 2019-02-22 13:56 |
| > Sure thing. Should it be discussed more detailed before the PR? If the document or the returned value is controversial, I would like to contribute if needed. Thanks :) Please, make the comments on the PR. This way, other reviewer can see your comments. |
|
|
| msg336321 - (view) |
Author: Julien Palard (mdk) *  |
Date: 2019-02-22 15:09 |
| Behavior changed in commit c9070d03f5169ad6e171e641b7fa8feab18bf229 (Jan 2018) but has been documented as returning None since 8ebeb03740dad4d9edd65de88f82840a05070941 (Jul 2014). |
|
|
| msg336327 - (view) |
Author: Kevin Mai-Hsuan Chia (mhchia) * |
Date: 2019-02-22 16:14 |
| @mdk That's interesting. May I ask which one is the preferred behavior, `None` or `[]`? IMO both make sense, but `None` is more consistent with the behavior in previous versions. If it is the case, may I work on fixing this(`None` to `[]`) as a practice to contribute? Otherwise, the PR from @eamanu should be enough to fix the document. Thanks a lot for the review and the pointer! |
|
|
| msg336328 - (view) |
Author: Emmanuel Arias (eamanu) * |
Date: 2019-02-22 16:25 |
| > @mdk That's interesting. May I ask which one is the preferred behavior, `None` or `[]`? IMO changing the returned of the sockets could represent a big change, and I think that this need to be discuss |
|
|
| msg336334 - (view) |
Author: miss-islington (miss-islington) |
Date: 2019-02-22 17:34 |
| New changeset df5cdc11123a35065bbf1636251447d0bfe789a5 by Miss Islington (bot) (Emmanuel Arias) in branch 'master': bpo-36074: Result of `asyncio.Server.sockets` after `Server.close()` after is not clear (GH-11987) https://github.com/python/cpython/commit/df5cdc11123a35065bbf1636251447d0bfe789a5 |
|
|
| msg336335 - (view) |
Author: Julien Palard (mdk) * |
Date: 2019-02-22 17:36 |
| Thanks Emmanuel for the PR (yes those PRs do not need NEWS). Thanks Kevin for the proposition to try fixing it the other way around, that's appreciated, don't hesitate to filter on easy issues there's some others :) |
|
|