Implement base protocol class #2986

asvetlov · 2018-05-08T13:05:16Z

To avoid a dependency from internal asyncio.streams.FlowControlMixin

kxepal · 2018-05-08T13:08:18Z

To avoid a dependency from internal asyncio.streams.FlowControlMixin

Why to avoid this? Do you expect that internals will change in some breaking way soon?

asvetlov · 2018-05-08T13:13:35Z

The class is not a part of public API. Technically it should be prefixed by underscore but we've missed it.

The second reason is type hinting: we have a public .transport property and expose our protocols in our public API (not sure if we should do it but the change can be done in 4.0 only).
Marking a property as "returning asyncio.Protocol" crashes mypy.

kxepal · 2018-05-08T13:29:46Z

Got it. Thanks!

kxepal · 2018-05-08T13:32:15Z

aiohttp/base_protocol.py

+        self._paused = False
+        self._drain_waiter = None
+        self._connection_lost = False
+        self.transport = None


Do we want transport as public attribute or instead public read-only property? I think transport is only valid to set via connection_made method, not somehow adhoc'y.

I'm trying don't touch transport attr, many aiohttp codes use it internally.
Tests modify .transport wildly as well.
Better to deprecate .protocol at all in our public API.

codecov-io · 2018-05-08T13:37:43Z

Codecov Report

Merging #2986 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2986      +/-   ##
==========================================
+ Coverage   98.08%   98.09%   +0.01%     
==========================================
  Files          41       42       +1     
  Lines        7621     7665      +44     
  Branches     1324     1335      +11     
==========================================
+ Hits         7475     7519      +44     
  Misses         49       49              
  Partials       97       97

Impacted Files	Coverage Δ
aiohttp/base_protocol.py	`100% <100%> (ø)`
aiohttp/client_proto.py	`94.16% <100%> (-0.24%)`	⬇️
aiohttp/web_protocol.py	`91.78% <100%> (-0.09%)`	⬇️

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 e0378bd...1519745. Read the comment docs.

kxepal

👍

This PR updates [aiohttp](https://pypi.org/project/aiohttp) from **3.2.1** to **3.3.1**. <details> <summary>Changelog</summary> ### 3.3.0 ``` ================== Features -------- - Raise ``ConnectionResetError`` instead of ``CancelledError`` on trying to write to a closed stream. (`2499 <https://github.com/aio-libs/aiohttp/pull/2499>`_) - Implement ``ClientTimeout`` class and support socket read timeout. (`2768 <https://github.com/aio-libs/aiohttp/pull/2768>`_) - Enable logging when ``aiohttp.web`` is used as a program (`2956 <https://github.com/aio-libs/aiohttp/pull/2956>`_) - Add canonical property to resources (`2968 <https://github.com/aio-libs/aiohttp/pull/2968>`_) - Forbid reading response BODY after release (`2983 <https://github.com/aio-libs/aiohttp/pull/2983>`_) - Implement base protocol class to avoid a dependency from internal ``asyncio.streams.FlowControlMixin`` (`2986 <https://github.com/aio-libs/aiohttp/pull/2986>`_) - Cythonize ``helpers.reify``, 5% boost on macro benchmark (`2995 <https://github.com/aio-libs/aiohttp/pull/2995>`_) - Optimize HTTP parser (`3015 <https://github.com/aio-libs/aiohttp/pull/3015>`_) - Implement ``runner.addresses`` property. (`3036 <https://github.com/aio-libs/aiohttp/pull/3036>`_) - Use ``bytearray`` instead of a list of ``bytes`` in websocket reader. It improves websocket message reading a little. (`3039 <https://github.com/aio-libs/aiohttp/pull/3039>`_) - Remove heartbeat on closing connection on keepalive timeout. The used hack violates HTTP protocol. (`3041 <https://github.com/aio-libs/aiohttp/pull/3041>`_) - Limit websocket message size on reading to 4 MB by default. (`3045 <https://github.com/aio-libs/aiohttp/pull/3045>`_) Bugfixes -------- - Don't reuse a connection with the same URL but different proxy/TLS settings (`2981 <https://github.com/aio-libs/aiohttp/pull/2981>`_) - When parsing the Forwarded header, the optional port number is now preserved. (`3009 <https://github.com/aio-libs/aiohttp/pull/3009>`_) Improved Documentation ---------------------- - Make Change Log more visible in docs (`3029 <https://github.com/aio-libs/aiohttp/pull/3029>`_) - Make style and grammar improvements on the FAQ page. (`3030 <https://github.com/aio-libs/aiohttp/pull/3030>`_) - Document that signal handlers should be async functions since aiohttp 3.0 (`3032 <https://github.com/aio-libs/aiohttp/pull/3032>`_) Deprecations and Removals ------------------------- - Deprecate custom application's router. (`3021 <https://github.com/aio-libs/aiohttp/pull/3021>`_) Misc ---- - 3008, 3011 ``` </details> <details> <summary>Links</summary> - PyPI: https://pypi.org/project/aiohttp - Changelog: https://pyup.io/changelogs/aiohttp/ - Repo: https://github.com/aio-libs/aiohttp </details>

lock · 2019-10-28T05:04:40Z

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].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new

asvetlov added 5 commits May 7, 2018 15:54

Make it work

e6b937f

Add missing file

16e37a0

Merge branch 'master' into base-protocol

b6d73f7

Test cover

1853dd3

Add changenote

1519745

kxepal reviewed May 8, 2018

View reviewed changes

kxepal approved these changes May 8, 2018

View reviewed changes

asvetlov merged commit ec6cfae into master May 8, 2018

asvetlov deleted the base-protocol branch May 8, 2018 14:05

dependencies bot mentioned this pull request Jun 2, 2018

aiohttp versions available: 3.3.0 Harmon758/Harmonbot#165

Closed

lock bot added the outdated label Oct 28, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019

psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Oct 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement base protocol class #2986

Implement base protocol class #2986

asvetlov commented May 8, 2018

kxepal commented May 8, 2018

asvetlov commented May 8, 2018

kxepal commented May 8, 2018

kxepal May 8, 2018

asvetlov May 8, 2018

kxepal May 8, 2018

codecov-io commented May 8, 2018 •

edited

Loading

kxepal left a comment

lock bot commented Oct 28, 2019

Implement base protocol class #2986

Implement base protocol class #2986

Conversation

asvetlov commented May 8, 2018

kxepal commented May 8, 2018

asvetlov commented May 8, 2018

kxepal commented May 8, 2018

kxepal May 8, 2018

Choose a reason for hiding this comment

asvetlov May 8, 2018

Choose a reason for hiding this comment

kxepal May 8, 2018

Choose a reason for hiding this comment

codecov-io commented May 8, 2018 • edited Loading

Codecov Report

kxepal left a comment

Choose a reason for hiding this comment

lock bot commented Oct 28, 2019

codecov-io commented May 8, 2018 •

edited

Loading