edaniszewski
commented
3 years ago Yeah, so just to provide some context:
- the app initializing bits were updated so
appis now global and not created by fn, as unit tests were bombing because they were being registered with the same name, and it looks like that now causes sanic errors - the sanic test client is being removed from the main sanic code where it previously resided, and the client methods now return a request and response, hence the proliferation of
_, resp = ...in test files - sanic changes how data is stored internally so there are updates to how we get internal info for tests, such as registered routes
- we can no longer call functions directly which are decorated by the
@routedecorator, as it now returns a tuple of metadata, so the/devices/...route had to be updated accordingly. - structlog also changed some internals with how contextvars work, so tests had to be updated to reflect that
I think thats most of it. Probably some additional small things.
This PR:
I'm not overall pleased with the latest sanic updates because a lot has changed, and while the major version was bumped accordingly, some of these changes took a lot of effort to track down what was happening. while I'm sure there was good reason for the changes, they do step on how some of our tests were set up. I think longer term, its probably worth moving to fastapi, if only for the better openapi support, but thats neither here nor there.
The driver for this PR was to update pyyaml so we are using the version which fixes the CVE it has.
A future PR will update how dependency management is done here so we can have more fine-grained control over updates.
fixes #418
Note: I will not cut a new release of Synse Server after this because I still want to do some local testing to make sure nothing else is funky. Expect a new release for this in the coming weeks.