Skip to content

Conversation

Swamp-Ig
Copy link
Contributor

Description:

Update to aiohttp version 3.4.0

Related issue (if applicable): fixes aio-libs/aiohttp#3208

Why this is important

I discovered this problem when interacting with a device I'm aiming on integrating.

The aiohttp client, which in turn relies on nodejs/http-parser, messes up the TCP ACK packets and results in a bunch of TCP errors after sending a HTTP message when the header is split between TCP packets.

This might not always be visible, however it does result in a bunch of extra network traffic. In my particular case it was visible because the device didn't change states if the TCP connection didn't close properly (which seems to be a bug to me, but anyhow).

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

@Swamp-Ig Swamp-Ig requested a review from a team as a code owner August 26, 2018 06:07
@homeassistant
Copy link
Contributor

Hi @Swamp-Ig,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@homeassistant homeassistant added cla-needed core small-pr PRs with less than 30 lines. labels Aug 26, 2018
@ghost ghost added the in progress label Aug 26, 2018
@fabaff
Copy link
Member

fabaff commented Aug 26, 2018

On Fedora 29 aiohttp-3.4.0 leads to a build error because of the changes around nodejs/http-parser.

creating build/temp.linux-x86_64-3.7/vendor/http-parser
gcc -pthread -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DNDEBUG -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -D_GNU_SOURCE -fPIC -fwrapv -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fPIC -DHTTP_PARSER_STRICT=0 -I/usr/include/python3.7m -c aiohttp/_http_parser.c -o build/temp.linux-x86_64-3.7/aiohttp/_http_parser.o
aiohttp/_http_parser.c:590:10: fatal error: ../vendor/http-parser/http_parser.h: No such file or directory
 #include "../vendor/http-parser/http_parser.h"
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
error: command 'gcc' failed with exit status 1

No sure how this affects the PyPI package.

@balloob
Copy link
Member

balloob commented Aug 26, 2018

@fabaff http-parser is a git submodule, so make sure you have submodules initialized, like as described in the linked aiohttp issue.

@balloob balloob added this to the 0.77 milestone Aug 26, 2018
@balloob
Copy link
Member

balloob commented Aug 26, 2018

@Swamp-Ig great find on the TCP issue. Will have it included in 0.77

@balloob balloob merged commit 69d104b into home-assistant:dev Aug 26, 2018
@ghost ghost removed the in progress label Aug 26, 2018
balloob pushed a commit that referenced this pull request Aug 26, 2018
@Swamp-Ig Swamp-Ig deleted the update-aiohttp-3.4.0 branch August 27, 2018 20:51
@balloob balloob mentioned this pull request Aug 29, 2018
girlpunk pushed a commit to girlpunk/home-assistant that referenced this pull request Sep 4, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Dec 10, 2018
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.

TCP not closing properly after POST
4 participants