chore: migrate to postgres and postgREST #263

hugomrdias · 2021-07-22T11:16:14Z

This PR now holds the postgres migration code in the /packages/api/db.

The packages/db folder still has the old fauna db code and will be deleted in the future.

Refer to #331 for more info on the migration plan.

cloudflare-workers-and-pages · 2021-07-22T11:19:01Z

Deploying with Cloudflare Pages

Latest commit:	`e5a575c`
Status:	✅ Deploy successful!
Preview URL:	https://d8aaa9be.nft-storage.pages.dev

View logs

alanshaw

🙏 comments and requests so far:

I need to be able to review the PR and it's just too big currently.
- Please can we have a separate PR for cf-sync against this branch?
- Can we please remove the fauna stuff? We don't need it right?
- I don't understand why we have a duplicated website? Can it be avoided/minimised?
Is a way we can just refactor the API code to use a database "client", and then pick the right one depending on the version number in the route URL?
I don't believe we can enforce a foreign key constraint on the dagcargo aggregate_entries table to our content table because it refers to CIDs in web3.storage also. We may have to write stored procedures for retrieving the data we need when deals info is involved.
Can we remove references to superbase in our code (aside from importing the client from the @superbase namespace)? As far as I can tell we're just using postgREST
supabase.ts please can we not mix typscript and javascript!

hugomrdias · 2021-09-05T19:36:40Z

🙏 comments and requests so far:

I need to be able to review the PR and it's just too big currently.

Please can we have a separate PR for cf-sync against this branch?

i would ask you to just ignore all changes inside tools for now, i will tidy up asap

Can we please remove the fauna stuff? We don't need it right?

done

I don't understand why we have a duplicated website? Can it be avoided/minimised?

Some stuff is a bit different specially in the auth keys pages, because now we can simplify a bunch. But i will reduce the duplication where possible.

Is a way we can just refactor the API code to use a database "client", and then pick the right one depending on the version number in the route URL?

We could but it gets a bit messy, i tried this already and changed to duplication.

I don't believe we can enforce a foreign key constraint on the dagcargo aggregate_entries table to our content table because it refers to CIDs in web3.storage also. We may have to write stored procedures for retrieving the data we need when deals info is involved.

we should, we just need to reference to join

Can we remove references to superbase in our code (aside from importing the client from the @superbase namespace)? As far as I can tell we're just using postgREST

done

supabase.ts please can we not mix typscript and javascript!

im using .ts because the types i generate from the openapi endpoint are really hard to use with jsdocs

this.client.from<definitions['account']>('account').upsert(account)

alanshaw · 2021-09-06T09:09:44Z

Some stuff is a bit different specially in the auth keys pages, because now we can simplify a bunch. But i will reduce the duplication where possible.

We don't want differences! Just minimal changes to make this work and give me a fighting chance of reviewing the PR properly.

Is a way we can just refactor the API code to use a database "client", and then pick the right one depending on the version number in the route URL?

We could but it gets a bit messy, i tried this already and changed to duplication.

How come it got messy? The duplication is difficult to review because I can't tell what's new/old.

I don't believe we can enforce a foreign key constraint on the dagcargo aggregate_entries table to our content table because it refers to CIDs in web3.storage also. We may have to write stored procedures for retrieving the data we need when deals info is involved.

we should, we just need to reference to join

Please explain?

supabase.ts please can we not mix typscript and javascript!

im using .ts because the types i generate from the openapi endpoint are really hard to use with jsdocs
this.client.from<definitions['account']>('account').upsert(account)

Understood but if we want to typescript things then we need a planned effort to do that. If we let this in it'll set precedence for a mixture and we'll not be able to spend time on it because what we have will be good enough and we'll end up doing other bits peicemeal. We'll then spend an extended period with a messy mixed code base and may never finish.

I'm not saying it's undesirable, just we should do it later. Create a PR, open an issue and we'll get it in.

hugomrdias · 2021-09-06T09:29:56Z

Regarding the duplication:

in next.js, it rely on the file system structure to build urls and going into the dynamic routes would make it difficult to use the static export
in the api basically the same thing, we could just setup v1 routes and point to the same files but the code would fork in multiple points or i would need to build a new abstraction for the old code to match the new db client.

the way i do this is, review in vscode with the github extension and just put the v0 and v1 files next to each other.

Regarding fdw:
we should be able to join because we are not actually enforcing anything just joining by reference (cid) so it doesnt matter if dag_cargo has cid (from web3) that we dont have.

i will remove typescript specific stuff and make everything js/jsdoc

hugomrdias · 2021-09-06T09:38:44Z

also i feel safer when deploy to production with the duplication, but i will try to merge everything at least in the website

hugomrdias · 2021-09-06T09:59:38Z

bah, nextjs doesnt support optional dynamic routes /login and /v1/login as [[version]]/login

I've taken the schema from #263 with the following changes: * Primary keys are called `id` except where we're using domain data as the primary key (currently only `cid` in content) * Use shortened primary key definition to use `BIGSERIAL` for enhanced readability * Added `nftstorage` schema * Renamed `account` -> `user` * Make `issuer` NOT the primary key on `user`, added `id` column for primary and referenced it from other tables * Prefixed foreign keys with (complete) referenced table name where appropriate * Renamed `cid` in the `upload` table to `source_cid` so more obvious it is the given/original/provided/source CID * Added the `deleted_at` column to `upload` (and had to remove the `UNIQUE` constraint because of it) * Consistent enums capitalization to match web3.storage and niftysave * Added `upload_type` enum * Renamed `content.size` to `content.dag_size` for consistency with web3.storage

alanshaw

I've opened #395 so we can collaborate on the schema. It includes the changes in the comments.

packages/api/db/tables.sql

hugomrdias · 2021-09-08T17:20:14Z

@alanshaw all changes in #395 already ported to this PR

all future changes to the schema should be easy to refactor in the db-client.js file and everything else should be caught by the type checker.

alanshaw

Not finished, but submitting feedback so far.

Note for me: I got down to packages/api/src/utils/db-client-types.ts

packages/api/src/utils/router.js

packages/api/src/utils/utils.js

packages/api/test/nfts-store-v1.spec.js

packages/api/db/functions.sql

packages/api/src/routes-v1/pins-add.js

packages/api/src/routes-v1/tokens-create.js

packages/api/src/routes-v1/pins-list.js

packages/api/src/routes-v1/nfts-list.js

packages/client/src/lib.js

packages/api/src/utils/utils.js

packages/api/src/utils/db-client.js

packages/client/src/lib/interface.ts

alanshaw

Ok, I'm done reviewing for now.

I've ignored the cf-sync and cron changes. I believe they are coming in a different PR? Can we remove from here?

I've also not really looked at the website stuff. I'm still not sure I understand why the need to copy functionality? There should be no API changes, why can't we just build with a different API endpoint?

packages/api/db/functions.sql

packages/client/test/service.js

alanshaw · 2021-09-17T15:05:15Z

@hugomrdias remember to merge this with feat:/fix: so it gets picked up by release please.

hugomrdias requested a review from alanshaw July 22, 2021 11:16

hugomrdias force-pushed the feat/migration1 branch from 5fd44b0 to b4356c6 Compare August 12, 2021 14:26

hugomrdias linked an issue Aug 17, 2021 that may be closed by this pull request

Database Migration #181

Closed

12 tasks

hugomrdias changed the title ~~chore: migrate to fauna~~ chore: migrate to postgres and postgREST Sep 1, 2021

hugomrdias force-pushed the feat/migration1 branch from 08c48bf to e371948 Compare September 1, 2021 11:08

alanshaw reviewed Sep 3, 2021

View reviewed changes

alanshaw mentioned this pull request Sep 6, 2021

feat: postgres schema #395

Closed

alanshaw requested changes Sep 6, 2021

View reviewed changes

packages/api/db/tables.sql Outdated Show resolved Hide resolved

packages/api/db/tables.sql Outdated Show resolved Hide resolved

hugomrdias marked this pull request as ready for review September 9, 2021 18:31

hugomrdias mentioned this pull request Sep 10, 2021

Migrate NFT.Storage to Postgres #331

Closed

19 tasks

alanshaw requested changes Sep 14, 2021

View reviewed changes

packages/api/src/utils/utils.js Show resolved Hide resolved

packages/api/src/utils/db-client.js Show resolved Hide resolved

packages/api/src/utils/db-client.js Show resolved Hide resolved

packages/client/src/lib/interface.ts Outdated Show resolved Hide resolved

alanshaw requested changes Sep 15, 2021

View reviewed changes

packages/api/db/functions.sql Outdated Show resolved Hide resolved

packages/client/test/service.js Show resolved Hide resolved

hugomrdias force-pushed the feat/migration1 branch from 922b881 to c17d230 Compare September 21, 2021 15:27

hugomrdias requested a review from alanshaw September 21, 2021 15:45

alanshaw approved these changes Sep 22, 2021

View reviewed changes

hugomrdias added 5 commits September 22, 2021 11:15

feat: new schema

25fcbb7

chore: more stuff

76957ce

chore: ignore ts

5e57f8c

feat: tokens create and delete

0598f2c

chore: schema for pin ingestion

3831f58

hugomrdias added 22 commits September 22, 2021 11:15

fix: add sdk

e177d8f

chore: fix tests

d2004a7

chore: more endpoints

91a305e

chore: change createUpload to createUploadCustom

e695a95

chore: list uploads

c7a6ab1

chore: store endpoint

e703a8d

feat: postgres migration

916e97b

chore: lock

a4c1eb7

chore: add last endpoints and change name to database

1711dae

chore: remove fauna stuff

8a0abd3

chore: remove more fauna stuff

5cecf6d

chore: update schema

3ae7411

feat: endpoints update to new schema and cid normalization

3b9454c

chore: enable tests in the ci

3f3152c

chore: fix client test

8dc5085

chore: fix website test

856cee8

chore: review requests

162665b

fix: dedupe website (#436)

895fcef

feat: migration deals schema (#418)

2c24182

chore: minor tweaks

fcf0c71

chore: lock

42bca51

chore: lock

05ac09f

hugomrdias force-pushed the feat/migration1 branch from e5a575c to 05ac09f Compare September 22, 2021 10:18

hugomrdias merged commit ff0c919 into main Sep 22, 2021

hugomrdias deleted the feat/migration1 branch September 22, 2021 10:24

This was referenced Feb 8, 2022

chore(main): release client 6.0.0 #1294

Closed

chore(main): release nft.storage 6.0.0 #1290

Closed

chore(main): release nft.storage 6.0.0 #1300

Closed

chore(main): release nft.storage 6.0.0 #1303

Closed

chore(main): release nft.storage 6.0.0 #1305

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: migrate to postgres and postgREST #263

chore: migrate to postgres and postgREST #263

hugomrdias commented Jul 22, 2021 •

edited

Loading

cloudflare-workers-and-pages bot commented Jul 22, 2021 •

edited

Loading

alanshaw left a comment

hugomrdias commented Sep 5, 2021

alanshaw commented Sep 6, 2021 •

edited

Loading

hugomrdias commented Sep 6, 2021 •

edited

Loading

hugomrdias commented Sep 6, 2021

hugomrdias commented Sep 6, 2021

alanshaw left a comment

hugomrdias commented Sep 8, 2021

alanshaw left a comment

alanshaw left a comment

alanshaw commented Sep 17, 2021

chore: migrate to postgres and postgREST #263

chore: migrate to postgres and postgREST #263

Conversation

hugomrdias commented Jul 22, 2021 • edited Loading

cloudflare-workers-and-pages bot commented Jul 22, 2021 • edited Loading

Deploying with Cloudflare Pages

alanshaw left a comment

Choose a reason for hiding this comment

hugomrdias commented Sep 5, 2021

alanshaw commented Sep 6, 2021 • edited Loading

hugomrdias commented Sep 6, 2021 • edited Loading

hugomrdias commented Sep 6, 2021

hugomrdias commented Sep 6, 2021

alanshaw left a comment

Choose a reason for hiding this comment

hugomrdias commented Sep 8, 2021

alanshaw left a comment

Choose a reason for hiding this comment

alanshaw left a comment

Choose a reason for hiding this comment

alanshaw commented Sep 17, 2021

hugomrdias commented Jul 22, 2021 •

edited

Loading

cloudflare-workers-and-pages bot commented Jul 22, 2021 •

edited

Loading

alanshaw commented Sep 6, 2021 •

edited

Loading

hugomrdias commented Sep 6, 2021 •

edited

Loading