pythonPackages.sanic: 19.3.1 -> 19.6.3 (and fix websockets and other issues) #68321

simonchatts · 2019-09-08T19:27:22Z

Motivation for this change

In pythonPackages, the sanic package requires websockets version 6.x, but the default provided is version 7.0.

The current workaround is just to delete the version bounds on this dependency. This moves the problem to runtime: the first time the websocket functionality is actually used, it fails (see below for a test case and resulting crash).

The change here brings in the correct version of the websockets Python package, so that this works.

Things done

Test case

In case useful, here's a minimal concrete test case.

If this is server.py:

from sanic import Sanic
from sanic.websocket import WebSocketProtocol

app = Sanic()

@app.websocket('/ws')
async def foo(req, ws):
  await ws.send('hello from websocket!')

app.run(protocol=WebSocketProtocol)

and you run this server via

nix-shell --pure -p 'python3.withPackages(p: [p.sanic])' --command 'python server.py'

(along with an assumed -I nixpkgs=<path to nixpkgs>), and invoke a test client:

nix-shell -p websocat --command 'websocat ws://127.0.0.1:8000/ws'

then with the current code, you get a crashdump:

[2019-09-08 19:35:50 +0100] [1688] [ERROR] Exception occurred while handling uri: 'ws://127.0.0.1:8000/ws'
Traceback (most recent call last):
  File "/nix/store/034m3rj9j27zvdldvpn6bdzpm0h3fn7c-python3-3.7.4-env/lib/python3.7/site-packages/sanic/app.py", line 917, in handle_request
    response = await response
  File "/nix/store/034m3rj9j27zvdldvpn6bdzpm0h3fn7c-python3-3.7.4-env/lib/python3.7/site-packages/sanic/app.py", line 473, in websocket_handler
    ws = await protocol.websocket_handshake(request, subprotocols)
  File "/nix/store/034m3rj9j27zvdldvpn6bdzpm0h3fn7c-python3-3.7.4-env/lib/python3.7/site-packages/sanic/websocket.py", line 69, in websocket_handshake
    key = handshake.check_request(request.headers)
  File "/nix/store/034m3rj9j27zvdldvpn6bdzpm0h3fn7c-python3-3.7.4-env/lib/python3.7/site-packages/websockets/handshake.py", line 82, in check_request
    [parse_connection(value) for value in headers.get_all('Connection')], []
AttributeError: 'multidict._multidict.CIMultiDict' object has no attribute 'get_all'

With the code changes here, you just see the correct client output:

hello from websocket!

Notify maintainers

cc @costrouc

FRidh

Multiple versions of a Python package in the Python packages set is not allowed.

FRidh · 2019-09-08T19:37:09Z

Maybe bumping the version of sanic is a solution.

simonchatts · 2019-09-08T21:11:22Z

Ah yes, makes sense now you say it. The very latest release of sanic updates to the version of websockets in nixpkg - I’ll give it a test, and update this PR if it seems stable. Thanks for the feedback!

simonchatts · 2019-09-09T19:37:34Z

OK, I've updated sanic to the latest release. This required several other cascading changes:

The newer version of sanic now depends on requests-async, which in turn depends on httpcore. These aren't in nixpkgs, but they are also marked as deprecated by their author. So rather than introduce an already-dying maintenance burden, I've declared them just as local packages within sanic.
The h11 Python package in the dependency graph was already broken for Python 3, so fixed that (by applying a one-line patch from their repo that has yet to be released).
The uvicorn framework is used for sanic testing, and this was pinned to an old version of uvloop. I've updated this, and tested with all the other packages that depend on uvicorn (edit: this is fastapi and starlette as well as sanic, where I tested a minimal web server for each).

I've tested the result on both nixos and macOS, and everything looks good so far.

lheckemann · 2019-09-10T12:20:42Z

Is this a fix that should be backported to 19.09?

simonchatts · 2019-09-11T08:51:22Z

@lheckemann I assume this is tradeoff judgment aimed at the reviewers and code owners. But I can say that without some sort of changes like these, in 19.09:

h11 is broken for python3 (and hence dependent packages like uvicorn, fastapi, starlette and wsproto)
sanic’s websockets functionality is broken

wd15 · 2019-09-11T17:53:51Z

The datasette tests don't seem to be passing with this change. However, I updated datasette to the lastest version and it builds and passes the tests (sanic is no longer a dependency). Here are the changes. Shall I try and add that into this PR or feel free to just duplicate?

simonchatts · 2019-09-11T19:46:01Z

Thanks @wd15! I've just duplicated as suggested to keep things simple.

I've also checked that the other python package that depends on sanic (pytest-sanic, in addition to datasette) at least builds with these changes.

wd15 · 2019-09-12T14:10:54Z

Also, mitmproxy seems to hang forever during the test stage as well as failing a few. It at least attempts to build now with these changes so is not a regression.

wd15 · 2019-09-16T15:28:53Z

After merging in the latest master (2ff3620) into this branch and resolving h11 conflicts nix-review passes so I'm giving my approval to this PR.

Result of nix-review 1 using Linux 5.0.0-27-generic #28-Ubuntu x86_64 GNU/Linux

5 packages were built:

datasette (python37Packages.datasette)
python37Packages.fastapi (python37Packages.datasette)
python37Packages.sanic (python37Packages.datasette)
python37Packages.starlette (python37Packages.datasette)
python37Packages.uvicorn (python37Packages.datasette)

It looks like the issues with mitmproxy have been resolved

zimbatm · 2019-09-19T11:23:01Z

it's rare to see so detailed reports in a PR. <3 @simonchatts

It looks like h11 has been fixed for python3 in master already. Do you mind rebasing the PR one last time?

simonchatts · 2019-09-19T13:30:43Z

Thanks @zimbatm - rebased with the existing h11 fix.

pkgs/development/python-modules/uvicorn/default.nix

Mic92 · 2019-09-23T19:51:11Z

looks there is a conflict again.

zimbatm

one last rebase?

pkgs/development/python-modules/uvicorn/default.nix

simonchatts · 2019-09-29T09:42:40Z

Thanks @zimbatm! I've done the rebase and suggested comment in my local repo, but hit a problem: @jonringer recently upgraded the Python websockets from 7.x to 8.x. Sanic requires 7.x.

I presume this sort of issue has come up before - any advice @wd15 @FRidh?

BTW @jonringer that websockets 8.x upgrade also breaks on Darwin, since it also enables the tests but they don't work on Darwin (hit OSError: AF_UNIX path too long) but that's a minor detail here.

jonringer · 2019-09-29T09:49:26Z

oops :( I got added to extra-users recently to ofborg, so I'll be more aware of when darwin fails

The new version depend on two packages that are not currently in nixpkgs, and marked as already deprecated by their author. So just include them locally. Also, adapt to the fact that the version of websockets in nixpkgs is 8.x, when sanic wants 7.x.

@wd51

Patch donated by @wd51, and removes sanic dependency.

simonchatts · 2019-09-29T20:21:33Z

I think I have a resolution to the major version conflict for websockets: the changes for websockets 8.x are actually very tame for sanic's usage, and they are already almost compatible.

So I've updated this branch with a small modification to sanic to use the 8.x websockets now in nixpkgs. (I'll also try to upstream these, so this should all be simpler in the future.) I've tested these with some apps that make heavy use of sanic websockets, and it all looks fine so far.

@jonringer I've also fixed the Darwin test issue with websockets here, but can obviously drop this if it's already covered by some other commit.

I think this resolves all the pending issues.

Mic92 · 2019-09-30T09:08:23Z

@GrahamcOfBorg build python37Packages.datasette datasette python37Packages.sanic python37Packages.uvicorn

simonchatts requested a review from FRidh as a code owner September 8, 2019 19:27

FRidh requested changes Sep 8, 2019

View reviewed changes

ofborg bot added the 6.topic: python label Sep 8, 2019

ofborg bot requested a review from costrouc September 8, 2019 19:43

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Sep 8, 2019

simonchatts force-pushed the sanic branch from cd4600d to fbe7850 Compare September 9, 2019 19:30

simonchatts changed the title ~~pythonPackages.sanic: provide correct websockets-6.x dependency~~ pythonPackages.sanic: 19.3.1 -> 19.6.3 (and fix websockets and other issues) Sep 9, 2019

ofborg bot requested a review from wd15 September 9, 2019 19:44

ofborg bot added 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 and removed 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Sep 9, 2019

simonchatts force-pushed the sanic branch from fbe7850 to 67ff3ec Compare September 9, 2019 20:08

simonchatts requested a review from FRidh September 9, 2019 22:24

lheckemann added this to the 20.03 milestone Sep 12, 2019

wd15 approved these changes Sep 16, 2019

View reviewed changes

simonchatts mentioned this pull request Sep 17, 2019

pythonPackages.entrance: init at 1.1.10 (and janus init at 0.4.0) #68957

Merged

10 tasks

simonchatts force-pushed the sanic branch from 290bd36 to 2a12886 Compare September 19, 2019 13:24

ofborg bot requested a review from wd15 September 19, 2019 13:47

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 and removed 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 labels Sep 19, 2019

wd15 approved these changes Sep 19, 2019

View reviewed changes

zimbatm reviewed Sep 21, 2019

View reviewed changes

pkgs/development/python-modules/uvicorn/default.nix Show resolved Hide resolved

zimbatm reviewed Sep 23, 2019

View reviewed changes

pkgs/development/python-modules/uvicorn/default.nix Show resolved Hide resolved

zimbatm reviewed Sep 23, 2019

View reviewed changes

pkgs/development/python-modules/uvicorn/default.nix Show resolved Hide resolved

simonchatts added 4 commits September 29, 2019 21:08

pythonPackages.uvicorn: fix propagated LICENSE.md

a3e33b6

pythonPackages.datasette: 0.28 -> 0.29.3

1cb9d4d

Patch donated by @wd51, and removes sanic dependency.

pythonPackages.websocket: disable failing tests on darwin

ee96f91

simonchatts force-pushed the sanic branch from 2a12886 to ee96f91 Compare September 29, 2019 20:08

ofborg bot requested a review from wd15 September 29, 2019 20:24

ofborg bot added 10.rebuild-darwin: 11-100 and removed 10.rebuild-darwin: 1-10 labels Sep 29, 2019

Mic92 merged commit 13ae344 into NixOS:master Sep 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pythonPackages.sanic: 19.3.1 -> 19.6.3 (and fix websockets and other issues) #68321

pythonPackages.sanic: 19.3.1 -> 19.6.3 (and fix websockets and other issues) #68321

simonchatts commented Sep 8, 2019

FRidh left a comment

FRidh commented Sep 8, 2019

simonchatts commented Sep 8, 2019

simonchatts commented Sep 9, 2019 •

edited

lheckemann commented Sep 10, 2019

simonchatts commented Sep 11, 2019

wd15 commented Sep 11, 2019

simonchatts commented Sep 11, 2019

wd15 commented Sep 12, 2019

wd15 commented Sep 16, 2019 •

edited

zimbatm commented Sep 19, 2019

simonchatts commented Sep 19, 2019

Mic92 commented Sep 23, 2019

zimbatm left a comment

simonchatts commented Sep 29, 2019

jonringer commented Sep 29, 2019

simonchatts commented Sep 29, 2019

Mic92 commented Sep 30, 2019

pythonPackages.sanic: 19.3.1 -> 19.6.3 (and fix websockets and other issues) #68321

pythonPackages.sanic: 19.3.1 -> 19.6.3 (and fix websockets and other issues) #68321

Conversation

simonchatts commented Sep 8, 2019

Motivation for this change

Things done

Test case

Notify maintainers

FRidh left a comment

Choose a reason for hiding this comment

FRidh commented Sep 8, 2019

simonchatts commented Sep 8, 2019

simonchatts commented Sep 9, 2019 • edited

lheckemann commented Sep 10, 2019

simonchatts commented Sep 11, 2019

wd15 commented Sep 11, 2019

simonchatts commented Sep 11, 2019

wd15 commented Sep 12, 2019

wd15 commented Sep 16, 2019 • edited

zimbatm commented Sep 19, 2019

simonchatts commented Sep 19, 2019

Mic92 commented Sep 23, 2019

zimbatm left a comment

Choose a reason for hiding this comment

simonchatts commented Sep 29, 2019

jonringer commented Sep 29, 2019

simonchatts commented Sep 29, 2019

Mic92 commented Sep 30, 2019

simonchatts commented Sep 9, 2019 •

edited

wd15 commented Sep 16, 2019 •

edited