Switch to sansio websockets back-end

[rwb27]

Uvicorn uses the now deprecated websockets.legacy back end by default. It also supports the newer sansio interface. As far as I can tell, either works, so I'm not sure why legacy is the default: backwards compatibility I guess.

I'd like to switch to the newer implementation now, before we are using websockets in anger, as it feels wrong to start out with deprecation warnings in the test suite! I don't believe we will notice the difference, but hopefully the test suite will tell. Websockets aren't used yet by the OFM.

[barecheck]

Barecheck - Code coverage report

Total: 96.34%

Your code coverage diff: 0.00% ▴

Uncovered files and lines

File	Lines
src/labthings_fastapi/server/cli.py	102, 184

[julianstirling]

This generally make sense. I think the worry is there will be inconsistent behaviour if people follow the example in tutorial/writing_a_thing.rst. Even though websockets are not in use it is probably worth updating that example.

This also relates to #263 as the OFM has its own uvicorn.run lines. I am not sure how best to wrap this so it can be enabled. But at the minimum I'd update the docs.

[rwb27]

This generally make sense. I think the worry is there will be inconsistent behaviour if people follow the example in tutorial/writing_a_thing.rst. Even though websockets are not in use it is probably worth updating that example.

This also relates to #263 as the OFM has its own uvicorn.run lines. I am not sure how best to wrap this so it can be enabled. But at the minimum I'd update the docs.

Thanks - I've updated the two places in the example code where I found uvicorn.run. I don't currently believe it's a problem if a different back-end is used, but obviously if we're testing against sansio it makes sense to use it elsewhere, and it's reasonable to be consistent.

I am starting to think that some sort of ThingServer.run() wrapper might be convenient: it's probably easier for people to find, and it means we can bundle in arguments that ought not to change. That would mean we change from:

uvicorn.run(server.app, port=5000, ws="websockets-sansio")

to

server.run(port=5000)

Maybe serve_with_uvicorn would be a clearer name for the function?

I could also see us wanting to add something like ThingServer.test_client or test_env for similar reasons.

I'm not proposing to add either in this PR, but it would be worth adding in a future one if you agree it's a reasonable way to go.

[julianstirling]

Thanks. I have made #298 for the final comment.