refactor(robot-server): Use a non-nullable indexed enum for protocol_kind

[SyntaxColoring]

Overview

Some tweaks to the protocol.protocol_kind SQL column, to throw in with database changes that we're doing anyway.

Changelog

• Use an enum (both in the SQL sense and in the Python sense), to stop us from storing bad values, and to make the whole Python side a bit more type-safe. Follows the pattern set in refactor(robot-server): move run time params to their own DB table #15650. Before, we were just using a plain str.
• Do not allow NULL values. NULL was semantically identical to ProtocolKind.STANDARD and was only stored by the 4->5 migration. Our older migration system could only add nullable=True columns, but that's no longer the case.
• Add an index so queries like "get all the quick-transfer protocols" or "count how many standard protocols there are" don't have to linearly scan the whole table. The higher-level code doesn't take advantage of this yet because it currently does its filtering in Python, but we should aim to move this kind of thing into SQL.

Database compatibility and merge sequencing

This intends to follow the plan started in #15650, which I understand as this:

The current public release is on schema 4. edge is on schema 5. #15650 abandons schema 5 in favor of 5.1. After #15650 merges, this PR, and a few others from @sanni-t, will merge in quick succession to modify 5.1 in-place. Then, a final PR will turn 5.1 into 6 and create a 5.0->6 migration. The aim of all of this is to make a few improvements on the database from what we have in 5, without introducing a migration for each of those improvements, and without tossing away the schema-5.0 data that's accumulated on all our internal robots.

Test Plan and Hands on Testing

• Run EXPLAIN QUERY PLAN in the sqlite3 CLI and make sure the index is working properly.
• Manually set up a dev server with an old database and inspect the SQL to make sure the migration happens properly.

Otherwise, I'm leaning on automated tests.

Review requests

• Am I understanding the merge sequencing plan correctly?
• Is the merge sequence stack already too precarious for us to put this on top of it, too?

Risk assessment

This will make the final PR in the plan, which finalizes the 5->6 split, slightly more complicated. It's one more change that we have to remember to handle correctly.

[vegano1]

Now that protocol_kind: Form default is ProtocolKind.STANDARD, do we need to set it here explicitly?

[SyntaxColoring]

It's weird. We should not, but apparently we do. See the TODO comments above.

[vegano1]

This cleans things up, and sets up the DB for faster access.
Thank you for putting this up!

[SyntaxColoring]

Discussed with @sanni-t: Will rebase and reimplement atop #15818, instead of merging into edge following #15650.

[sanni-t]

LGTM!

[sanni-t]

I think previously if the protocol kind specified in the request was not one of the defined kinds then it would raise an error. Are we intentionally ignoring it now?

[SyntaxColoring]

In the actual running server, FastAPI will still reject invalid protocolKinds.

        {
            "id": "InvalidRequest",
            "title": "Invalid Request",
            "detail": "value is not a valid enumeration member; permitted: 'standard', 'quick-transfer'",
            "source": {
                "pointer": "/protocolKind"
            },
            "errorCode": "4000"
        }

This is a weird unit-test-only conditional.

I think this can be fixed by adopting FastAPI's newer Annotated[...] syntax instead of the current = Depends(...) syntax, which I'm working on on the side.

[sanni-t]

Ah right right!

Ya, switching to Annotated would be nice.