FastAPI port of API/Studio, second time around

[dhdaines]

This time with backwards compatibility and testing! Wow!

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 98.63014% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 93.84%. Comparing base (7ec44b3) to head (9bc3855).

:exclamation: Current head 9bc3855 differs from pull request most recent head 2d68577. Consider uploading reports for the commit 2d68577 to get more accurate results

Files	Patch %	Lines
g2p/app.py	97.43%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #360 +/- ## ========================================== + Coverage 93.77% 93.84% +0.07% ========================================== Files 16 16 Lines 2328 2291 -37 Branches 517 516 -1 ========================================== - Hits 2183 2150 -33 + Misses 82 79 -3 + Partials 63 62 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[github-actions[bot]]

CLI load time: 0:00.05
PR head 2d6857776b0dd4bd0d2d8dd995c780ab60ea34fb
Imports that take more than 0.1 s:
import time: self [us] | cumulative | imported package

[dhdaines]

Note that fastapi-socketio is really broken, so it would probably be better to not use it: https://github.com/pyropy/fastapi-socketio/issues/52

In the end there's very little actual code in it so we can just use socketio directly.

[dhdaines]

Also note that this would mean that the API and Studio require Python 3.8, so not sure if we want to go there...

[joanise]

Also note that this would mean that the API and Studio require Python 3.8, so not sure if we want to go there...

The Web API already requires Python 3.8, we only continue to support 3.7 for the programmatic Python API, i.e., g2p as a Python module.

[dhdaines]

Also note that this would mean that the API and Studio require Python 3.8, so not sure if we want to go there...

The Web API already requires Python 3.8, we only continue to support 3.7 for the programmatic Python API, i.e., g2p as a Python module.

Ah, of course - then there's no problem since the API is a separate, optional dependency here. But we need to separate the test suites so that we can test the non-API parts with 3.7.

[dhdaines]

Also, not quite sure what's going wrong with coverage in CI here, it gives the correct results when I run it locally.

[joanise]

I believe this will be ready to merge, with #363 and a redirect link from /docs to /api/v1/docs, although when we also merge the v2, we might want a page listing both instead of an actual redirect...

[joanise]

Meh, I'm being bitten by a stale environment again. I'll create something fresh to test.

[joanise]

I hate dependency hell!

[dhdaines]

I hate dependency hell!

Hm, I'm really surprised this is happening ... I didn't have to change anything in my environment, the packages are the same as before the last updates except that we aren't using some of the ones we were previously...

[joanise]

Found it:

     "starlette>=0.35.1",
-    "python-socketio>=5",
+    "python-socketio>=5.9.0",
     "uvicorn",

[dhdaines]

Found it:

     "starlette>=0.35.1",
-    "python-socketio>=5",
+    "python-socketio>=5.9.0",
     "uvicorn",

aaah ... okay, I put python-socketio>=5 because that's what supports the same protocol as the JavaScript side, but I'll apply this fix (presuming there is no problem with Python 3.8)

[joanise]

That's the minimum required patch. On a stale environment with python-socketio==5.8.0 I got exceptions like this from ./run_studio.py:

WARNING:  No supported WebSocket library detected. Please use 'pip install uvicorn[standard]', or install 'websockets' or 'wsproto' manually.
INFO:     127.0.0.1:62614 - "GET /ws/socket.io/?EIO=4&transport=websocket HTTP/1.1" 500 Internal Server Error
ERROR:    Exception in ASGI application
Traceback (most recent call last):

A simpler fix was to install uvicorn[standard], but that installed quite a few more things. Just python-socketio==5.9.0 gave me simple-websocket==1.0.0 as a side effect and that was enough.

I suppose we could also declare simple-websocket ourselves in the api dependency block.

[joanise]

Warning: you accidentally undid #365. Maybe we need to make the change in pyproject.toml too. That fixes a prod CVE, so we really want to apply it.

[dhdaines]

Warning: you accidentally undid #365. Maybe we need to make the change in pyproject.toml too. That fixes a prod CVE, so we really want to apply it.

Oh, oops. This may be due to "git rebase"'s merge conflicts being backwards and hard to understand.

[dhdaines]

Warning: you accidentally undid #365. Maybe we need to make the change in pyproject.toml too. That fixes a prod CVE, so we really want to apply it.

Oh, oops. This may be due to "git rebase"'s merge conflicts being backwards and hard to understand.

I don't understand, though ... how did the updated gunicorn dependency disappear from the main branch?

[joanise]

Alright, finally I can say I agree: we're there! We can merge this.

Thank you for your patience working through all the details, and putting up with my stale environments which, while annoying, bring us to express our actual dependencies more accurately, so I don't actually feel bad about it. 😉

[joanise]

Oh, oops. This may be due to "git rebase"'s merge conflicts being backwards and hard to understand.

I don't understand, though ... how did the updated gunicorn dependency disappear from the main branch?

You almost certainly made a mistake in handling a merge conflict.

This is the commit that undoes it:

commit cfc50c6ef1
Author: David Huggins-Daines <dhd@ecolingui.ca>
Date:   Thu Apr 4 10:09:02 2024 -0400

    fix: update prod environment and workflow

[joanise]

In your defense, merge conflict resolution is indeed quite confusing. I'm quite paranoid about them, often doing extensive reviewing of the commit I'm trying to rebase and the changes that were there before, and not just relying on the conflict resolution tools.