[DX-123] seprated db and api schemas #1701

suhailkakar · 2023-04-26T08:32:13Z

What does this pull request do? Explain your changes. (required)

Separated database and API schemas. Instead of one schema, we now have two schemas, the api-schema.yaml is the schema for API references, and db-schema.yaml is similar to API schema except it has keys that open API doesn't validate but it is needed for db such as readOnly, table.

@victorges This PR is just for the separation of the schemas, I will open two other follow-up PRs for including other APIs that are missing here and another one for API references

Specific updates (required)

Separated database and API schemas

Does this pull request close any open issues?
Linear - DX 123

Checklist

I have read the CONTRIBUTING document.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.

vercel · 2023-04-26T08:32:18Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
livepeer-studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 4, 2023 8:22am

suhailkakar · 2023-04-26T08:38:28Z

I will take a look on why test are failing as well :)

hjpotter92 · 2023-04-26T08:40:55Z

Does this pull request close any open issues?
Linear - DX 123

you can edit the title of your PR to start with [DX-123] and linear should automatically pick up correct reference.

suhailkakar · 2023-04-26T08:51:50Z

Oh, that is cool! thanks @hjpotter92

linear · 2023-04-26T08:51:58Z

DX-123 OpenAPI cleanup

victorges

LGTM!

Made comments about components/APIs that should not be in the public API reference (api-schema.yaml). Looks like there are a lot of comments but it's just flagging of some components that should be removed from the API reference (either moved to the internal schema or deleted completely)

victorges · 2023-04-26T17:47:35Z

packages/api/src/schema/db-schema.yaml

+      table: webhook
+      properties:
+        id:
+          readOnly: true


readOnly is an OpenAPI field which changes the generated documentation (doesn't include the field in the request payloads, only responses). https://swagger.io/docs/specification/data-models/data-types/

Is this breaking the readme.io generation somehow? If that's the case, this is probably something to bring up with them as they are not following the OpenAPI spec.

victorges · 2023-04-26T17:49:21Z

packages/api/src/schema/db-schema.yaml

+            sourceInfo:
+              properties:
+                width:
+                  minValue: 1


Interesting, I though this was OpenAPI as well but their field is actually minimum and maximum. Maybe this never worked lol

packages/api/src/schema/db-schema.yaml

victorges · 2023-04-26T17:50:48Z

packages/api/src/schema/db-schema.yaml

+        recordingStatus:
+          readonly: true
+        recordingUrl:
+          readonly: true
+        mp4Url:
+          readonly: true


typo

Suggested change

recordingStatus:

readonly: true

recordingUrl:

readonly: true

mp4Url:

readonly: true

recordingStatus:

readOnly: true

recordingUrl:

readOnly: true

mp4Url:

readOnly: true

(should be moved to api-schema anyway, like all other readOnlys)

victorges · 2023-04-26T17:55:23Z

packages/api/src/schema/api-schema.yaml

+            - hevc
+            - vp8
+            - vp9
+    cdn-data-row:


This is an internal object type that should not be in api reference. We should move it to db-schema

victorges · 2023-04-26T18:02:12Z

packages/api/src/schema/api-schema.yaml

+            application/json:
+              schema:
+                $ref: "#/components/schemas/error"
+  /user/token:


Same, let's remove this

victorges · 2023-04-26T18:02:36Z

packages/api/src/schema/api-schema.yaml

+            application/json:
+              schema:
+                $ref: "#/components/schemas/error"
+  "/user/{id}":


Same, this. oneis internal. We can change it to /user/me though and re-document it as "retrieves own user object"

victorges · 2023-04-26T18:03:08Z

packages/api/src/schema/api-schema.yaml

+            application/json:
+              schema:
+                $ref: "#/components/schemas/error"
+  /api-token:


Same, let's remove this. Users can only use our API with an API key, and API keys are not allowed to manage other API keys for security purposes.

victorges · 2023-04-26T18:03:40Z

packages/api/src/schema/api-schema.yaml

+            application/json:
+              schema:
+                $ref: "#/components/schemas/error"
+  "/region/{region}":


Same, let's remove this

victorges · 2023-04-26T18:03:53Z

packages/api/src/schema/api-schema.yaml

+            application/json:
+              schema:
+                $ref: "#/components/schemas/error"
+  /room:


Nice! Good to know you already rebased 🙏

victorges · 2023-04-26T18:08:49Z

Hmm, it seems like the build is also failing. Is the merged schema coming out with the exact types as the original schema? Maybe one flow that you can follow is:

run yarn compile-schemas from master (from inside packages/api)
copy the generated schema.json file
change to your branch and run compile-schemas again
compare the generated schema.json with the one that came from master. Make sure only docs change

victorges

a couple documentation fixes

packages/api/src/schema/api-schema.yaml

victorges · 2023-04-26T22:27:56Z

packages/api/src/schema/api-schema.yaml

+          type: array
+          items:
+            $ref: "#/components/schemas/ffmpeg-profile"
+    error:


Ohhh interesting! We should use this schema for the error responses in the API declarations below. Will save a lot of repetitive declarations.

packages/api/src/schema/api-schema.yaml

victorges · 2023-04-26T22:43:32Z

packages/api/src/schema/api-schema.yaml

+      properties:
+        $ref: "#/components/schemas/signing-key/properties"
+        privateKey:
+          type: string


So this is a little hack we've used in our schema: a $ref key that is not the only entry in the object, and then we add more keys to it. Make sure this works with readme.io!

packages/api/src/schema/api-schema.yaml

Co-authored-by: Victor Elias <victor@livepeer.org>

victorges

Amazing! Only some smaller comments now. The only one that might involve some work is the readOnly, which might be easier to handle with a script instead of manually 🥴

packages/api/src/schema/db-schema.yaml

victorges · 2023-04-28T19:23:56Z

packages/api/src/schema/db-schema.yaml

+            Name of template to send to the users regarding the suspension.
+          enum:
+            - copyright
+    webhook:


I think this one was forgotten here with the full spec and not only the fields diff. This can be mostly on the public API reference on ly, we only need the internal fields here (e.g. table and the event property).
Nevermind, realized it was including only the internal fields already (only got the readonly issue)

Friendly reminder that the readOnly ones should go back to API reference (not sure if you intended to include that in this update, just making sure). Confirmed that readme.io supposedly supports those: https://docs.readme.com/main/discuss/6137da73509c3500180719ed

Thanks! Just pushed the changes for readOnly fields going back to api-schema

packages/api/src/schema/db-schema.yaml

packages/api/src/schema/api-schema.yaml

victorges · 2023-04-28T19:39:49Z

packages/api/src/schema/api-schema.yaml

+          type: object
+          description: User-defined webhook context
+          additionalProperties: true
+paths:


Not looking from here down as you mentioned you're updating this section later

victorges · 2023-04-28T19:41:50Z

(need to fix tests/build)

Edit: did it for you 🎁

adamsoffer · 2023-05-03T14:04:05Z

Looks great! Good to merge @suhailkakar?

suhailkakar · 2023-05-03T14:07:33Z

Almost!, just pushing a final change :)

adamsoffer · 2023-05-04T20:33:49Z

Almost!, just pushing a final change :)

great. feel free to merge afterwards

suhailkakar · 2023-05-05T06:06:31Z

I believe it is now completed, @victorges can you please confirm?

victorges

Yes!

adamsoffer · 2023-05-08T21:10:04Z

@victorges heads up not sure if you're aware but looks like checks are failing after merging.

victorges · 2023-05-09T16:39:20Z

Yeah that was fixed later on #1720. I think this branch wasn't rebased

seprated db and api schemas

9b2f461

suhailkakar requested a review from a team as a code owner April 26, 2023 08:32

suhailkakar changed the title ~~seprated db and api schemas~~ [DX-123] seprated db and api schemas Apr 26, 2023

victorges approved these changes Apr 26, 2023

View reviewed changes

victorges reviewed Apr 26, 2023

View reviewed changes

suhailkakar and others added 6 commits April 28, 2023 22:41

Update packages/api/src/schema/api-schema.yaml

6677bf9

Co-authored-by: Victor Elias <victor@livepeer.org>

Update packages/api/src/schema/api-schema.yaml

72b5e27

Co-authored-by: Victor Elias <victor@livepeer.org>

Update packages/api/src/schema/api-schema.yaml

1465e5a

Co-authored-by: Victor Elias <victor@livepeer.org>

Update packages/api/src/schema/api-schema.yaml

eeb79bc

Co-authored-by: Victor Elias <victor@livepeer.org>

Update packages/api/src/schema/api-schema.yaml

970820c

Co-authored-by: Victor Elias <victor@livepeer.org>

Update packages/api/src/schema/api-schema.yaml

d6d4c53

Co-authored-by: Victor Elias <victor@livepeer.org>

vercel bot had a problem deploying to Preview April 28, 2023 17:16 Failure

suhailkakar added 2 commits April 28, 2023 23:05

Update api-schema.yaml

91e69d8

Update db-schema.yaml

2e36b1f

vercel bot had a problem deploying to Preview April 28, 2023 17:41 Failure

Update db-schema.yaml

0c9a8ef

vercel bot had a problem deploying to Preview April 28, 2023 17:50 Failure

victorges approved these changes Apr 28, 2023

View reviewed changes

api/schema: Fix build and tests

3d34b1b

victorges force-pushed the api-reference branch from 9aa015e to 3d34b1b Compare April 28, 2023 20:30

vercel bot deployed to Preview April 28, 2023 20:37 View deployment

suhailkakar requested a review from a team as a code owner May 4, 2023 08:07

suhailkakar force-pushed the api-reference branch from 66b191f to 3d34b1b Compare May 4, 2023 08:18

removed readOnly fields from db-schema and added to api-schema

e8a9bcb

vercel bot deployed to Preview May 4, 2023 08:22 View deployment

victorges approved these changes May 8, 2023

View reviewed changes

suhailkakar merged commit 45cb66e into master May 8, 2023
12 checks passed

suhailkakar deleted the api-reference branch May 8, 2023 18:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DX-123] seprated db and api schemas #1701

[DX-123] seprated db and api schemas #1701

suhailkakar commented Apr 26, 2023 •

edited

vercel bot commented Apr 26, 2023 •

edited

suhailkakar commented Apr 26, 2023

hjpotter92 commented Apr 26, 2023 •

edited

suhailkakar commented Apr 26, 2023

linear bot commented Apr 26, 2023

victorges left a comment

victorges Apr 26, 2023

victorges Apr 26, 2023

victorges Apr 26, 2023

victorges Apr 26, 2023

victorges Apr 26, 2023

victorges Apr 26, 2023

victorges Apr 26, 2023

victorges Apr 26, 2023

victorges Apr 26, 2023

victorges commented Apr 26, 2023

victorges left a comment

victorges Apr 26, 2023

victorges Apr 26, 2023

victorges left a comment

victorges Apr 28, 2023 •

edited

suhailkakar May 4, 2023

victorges Apr 28, 2023

victorges commented Apr 28, 2023 •

edited

adamsoffer commented May 3, 2023

suhailkakar commented May 3, 2023

adamsoffer commented May 4, 2023

suhailkakar commented May 5, 2023

victorges left a comment

adamsoffer commented May 8, 2023

victorges commented May 9, 2023

[DX-123] seprated db and api schemas #1701

[DX-123] seprated db and api schemas #1701

Conversation

suhailkakar commented Apr 26, 2023 • edited

vercel bot commented Apr 26, 2023 • edited

suhailkakar commented Apr 26, 2023

hjpotter92 commented Apr 26, 2023 • edited

suhailkakar commented Apr 26, 2023

linear bot commented Apr 26, 2023

victorges left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

victorges commented Apr 26, 2023

victorges left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

victorges left a comment

Choose a reason for hiding this comment

victorges Apr 28, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

victorges commented Apr 28, 2023 • edited

adamsoffer commented May 3, 2023

suhailkakar commented May 3, 2023

adamsoffer commented May 4, 2023

suhailkakar commented May 5, 2023

victorges left a comment

Choose a reason for hiding this comment

adamsoffer commented May 8, 2023

victorges commented May 9, 2023

suhailkakar commented Apr 26, 2023 •

edited

vercel bot commented Apr 26, 2023 •

edited

hjpotter92 commented Apr 26, 2023 •

edited

victorges Apr 28, 2023 •

edited

victorges commented Apr 28, 2023 •

edited