feat: move access-api delegation bytes out of d1 and into r2 #578

gobengo · 2023-03-20T23:33:39Z

Motivation:

first PR on store access-api delegation bytes in R2 not D1 #571

todo

initial DbDelegationsStorageWithR2 that passes tests as a DelegationsStorage
rm unused access-ws package chore: remove access-ws and access-client package tests that rely on it #596
access-api should use DbDelegationsStorageWithR2 and not DbDelegationsStorage (without r2) and still pass tests feat: access-api env.models.delegations uses DbDelegationsStorageWithR2 #599

informed by review

add r2 bucket name binding to wrangler.toml for staging, production feat: move access-api delegation bytes out of d1 and into r2 #578 (comment)

unblocks:

DbDelegationsStorageWithR2 supports deletes
- we don't need this to support current DelegationsStorage, but probably will before long, but doesn't need to block this PR
optimization where r2 stores all the CIDs with varying multibases but for the same hash only once
safe to remove DbDelegationsStorage, DelegationsV2Row, DelegationsV2Tables

…ns across packages in this monorepo

packages/access-api/package.json

packages/access-api/src/models/delegations.js

…it (#596) Motivation: * #593 * by removing this, [my other PR](#578) doesn't have to worry about this package anymore (I had to when I encountered some miniflare/wrangler/workers-types/etc type mismatches when adopting R2Bucket type) Notes * this removes some tests in access-client that tested against this access-ws running in miniflare. If we need those back, we can always checkout the things we want back from these commits. Putting #awake here so it shows up in repo search results * My understanding is that the websocket used by access-client agent is different than the access-ws package implemented

gobengo · 2023-03-22T19:51:45Z

packages/access-api/src/models/delegations.js

+    if (delegations.length === 0) {
+      return
+    }
+    await writeDelegations(this.#dags, delegations)


We might succeed here, but then fail on the insert, in which case we will be storing more than is necessary in the R2Bucket, but I wrote this so we err on that side but at least avoid ever having delegation.cid in D1 but are unable to dereference CID -> bytes from r2.

If d1 had access to sql transactions, we might be able to use that here to do the write to r1 after trying the sql insert inside a transaction, but we don't so this is I think the best we can do without a much more involved 2pc
(but at that point IMHO we should be looking beyond d1, even to sql setup with transactions or away from sql entirely)

… upgrades (#598) Motivation: * https://github.com/web3-storage/w3protocol/pull/578/files#r1144141609

packages/access-api/wrangler.toml

alanshaw

LGTM but personally I would name the binding and bucket_name differently.

alanshaw · 2023-03-22T23:17:50Z

packages/access-api/src/bindings.d.ts

+   * primary r2 bucket used by access-api.
+   * will be used for storing env.models.delegations CARs
+   */
+  ACCESS_API_R2: R2Bucket


Just have a bucket specific to the things it's storing? We can create many of these - we don't have to have only 1 for everything we store.

Nit: I think @alanshaw suggesting this should be more like DELEGATIONS_BUCKET which I'd find to be more intuitive as well. I would also suggest not baking product names like R2 so it's not tied to CF product and could work e.g if we move to S3.

alanshaw · 2023-03-22T23:35:39Z

packages/access-api/wrangler.toml

+binding = "ACCESS_API_R2"
+bucket_name = "access-api-dev-1"


Proposal:

Suggested change

binding = "ACCESS_API_R2"

bucket_name = "access-api-dev-1"

binding = "DELEGATIONS"

bucket_name = "access-api-delegations-dev-0"

sgtm! I will create buckets following similar pattern for non-dev envs

Gozala

Mostly looks good, only have not approved it because I'd like R2 path changes so they are not misleading given current naming schema.

I would also highly encourage to remove legacy table and stores as they introduce unnecessary complexity not only here but also downstream. PRs that I'm working on interact with delegation store and now having to deal with two variants would further ripple that complexity. I don't think keeping both provides enough benefit to justify this.

Other than than I have added bunch of nits which you can address or ignore.

packages/access-api/migrations/0007_add_delegations_v3.sql

Gozala · 2023-03-22T23:43:28Z

packages/access-api/src/bindings.d.ts

+   * primary r2 bucket used by access-api.
+   * will be used for storing env.models.delegations CARs
+   */
+  ACCESS_API_R2: R2Bucket


Nit: I think @alanshaw suggesting this should be more like DELEGATIONS_BUCKET which I'd find to be more intuitive as well. I would also suggest not baking product names like R2 so it's not tied to CF product and could work e.g if we move to S3.

Gozala · 2023-03-22T23:47:53Z

packages/access-api/src/config.js

@@ -35,6 +35,12 @@ export function loadConfig(env) {
    }
  }

+  if (typeof env.ACCESS_API_R2 !== 'object') {
+    throw new TypeError(
+      `expected env.ACCESS_API_R2 to be an R2Bucket object, but got ${typeof env.ACCESS_API_R2}`


Nit: Would be nice if error said wrangler magic one supposed to do to fix this.

it shouldn't happen under normal circumstances, i.e. there is no wrangler magic to recommend. the wranger.toml will set this up in a good way for known envs. This would catch it if you somehow opted out of that calling this function.

packages/access-api/src/models/delegations.js

Gozala · 2023-03-23T00:25:44Z

packages/access-api/src/models/spaces.js

@@ -32,7 +32,7 @@ export class Spaces {
      inserted_at: (v) => new Date(v),
      updated_at: (v) => new Date(v),
    })
-    this.d1 = /** @type {Kysely<import('../bindings').D1Schema>} */ (
+    this.d1 = /** @type {Kysely<{ spaces: SpaceTable }>} */ (


Nit: Can you avoid changing this unless you need to as it would conflict with my changes in the other PR.

I looked and it should be easy to resolve any conflicts, and there may be none. This is just narrowing the type to only what's needed.
It also is paving way for moving kysely/database-related types from access-client (e.g. so we can remove access-client dep on kysely).

Gozala · 2023-03-23T00:26:29Z

packages/access-api/src/models/accounts.js

Can you avoid changing this unless needed as it would create conflict with my PR also changing this file.

see #578 (comment)

packages/access-api/test/delegations-storage.test.js

Gozala · 2023-03-23T00:31:23Z

packages/access-api/wrangler.toml

+binding = "ACCESS_API_R2"
+bucket_name = "access-api-dev-1"


Suggested change

binding = "ACCESS_API_R2"

bucket_name = "access-api-dev-1"

binding = "DELEGATIONS_BUCKET"

bucket_name = "delegations-bucket-dev-0"

I would suggest

bucket part seemes redundant, but I changed to w3up-delegations-{env}-{i}

…nger.toml to use env-specific buckets (#603) Motivation: * #578 (comment) * #578 (comment)

Co-authored-by: Irakli Gozalishvili <contact@gozala.io>

…elegationsStorageWithR2)

…egation

gobengo · 2023-03-23T07:14:36Z

I'd like R2 path changes so they are not misleading given current naming schema.

I think I addressed this in comments, but if I didn't and we want a different r2 key format, we can make a followup pr and test change without any overhead.

update: #617

🤖 I have created a release *beep* *boop* --- ## [5.0.0](access-api-v4.11.0...access-api-v5.0.0) (2023-03-23) ### ⚠ BREAKING CHANGES * implement new account-based multi-device flow ([#433](#433)) * upgrade capabilities to latest ucanto ([#463](#463)) ### Features * access-api handles provider/add invocations ([#462](#462)) ([5fb56f7](5fb56f7)) * access-api serves access/claim invocations ([#456](#456)) ([baacf35](baacf35)) * access/authorize confirmation email click results in a delegation back to the issuer did:key so that access/claim works ([#460](#460)) ([a466a7d](a466a7d)) * allow multiple providers ([#595](#595)) ([96c5a2e](96c5a2e)) * define `access/confirm` handler and use it in ucanto-test-utils registerSpaces + validate-email handler ([#530](#530)) ([b1bbc90](b1bbc90)) * handle access/delegate invocations without error ([#427](#427)) ([4f0bd1c](4f0bd1c)) * if POST /validate-email?mode=authorize catches error w/ too big qr code ([#516](#516)) ([d0df525](d0df525)) * implement new account-based multi-device flow ([#433](#433)) ([1ddc6a0](1ddc6a0)) * includes proofs chains in the delegated authorization chain ([#467](#467)) ([5144293](5144293)) * move access-api delegation bytes out of d1 and into r2 ([#578](#578)) ([4510c9a](4510c9a)) * move validation flow to a Durable Object to make it ⏩ fast ⏩ fast ⏩ fast ⏩ ([#449](#449)) ([02d7552](02d7552)) * provision provider type is now the DID of the w3s service ([#528](#528)) ([6a72855](6a72855)) * space/info will not error for spaces that have had storage provider added via provider/add ([#510](#510)) ([ea4e872](ea4e872)) * upgrade capabilities to latest ucanto ([#463](#463)) ([2d786ee](2d786ee)) * upgrade to new ucanto ([#498](#498)) ([dcb41a9](dcb41a9)) * write invocations and receipts into ucan log ([#592](#592)) ([754bf52](754bf52)) ### Bug Fixes * access/delegate checks hasStorageProvider(space) in a way that provider/add allows access/delegate ([#483](#483)) ([f4c640d](f4c640d)) * adjust migration 0005 to keep delegations table but create new used delegations_v2 ([#469](#469)) ([a205ad1](a205ad1)) * adjust migration 0005 to not do a drop table and instead rename delegations -> delegations_old and create a new delegations ([#468](#468)) ([6c8242d](6c8242d)) * allow injecting email ([#466](#466)) ([e19847f](e19847f)) * DbDelegationsStorage#find throws UnexpectedDelegation w/ { row } if failed bytesToDelegations ([#476](#476)) ([a6dafcb](a6dafcb)) * DbProvisionsStorage putMany doesnt error on cid col conflict ([#517](#517)) ([c1fea63](c1fea63)) * delegations model tries to handle if row.bytes is Array not Buffer (e.g. cloudflare) ([#478](#478)) ([030e7b7](030e7b7)) ### Miscellaneous Chores * **access-client:** release 11.0.0-rc.0 ([#573](#573)) ([be4386d](be4386d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release *beep* *boop* --- ## [11.0.0](access-v11.0.0-rc.0...access-v11.0.0) (2023-03-23) ### Features * move access-api delegation bytes out of d1 and into r2 ([#578](#578)) ([4510c9a](4510c9a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: Alan Shaw <alan.shaw@protocol.ai>

…it (#596) Motivation: * #593 * by removing this, [my other PR](#578) doesn't have to worry about this package anymore (I had to when I encountered some miniflare/wrangler/workers-types/etc type mismatches when adopting R2Bucket type) Notes * this removes some tests in access-client that tested against this access-ws running in miniflare. If we need those back, we can always checkout the things we want back from these commits. Putting #awake here so it shows up in repo search results * My understanding is that the websocket used by access-client agent is different than the access-ws package implemented

Motivation: * first PR on #571 todo * [x] initial `DbDelegationsStorageWithR2` that passes tests as a `DelegationsStorage` * [x] rm unused access-ws package #596 * [x] access-api should use `DbDelegationsStorageWithR2` and not `DbDelegationsStorage` (without r2) and still pass tests #599 informed by review * [x] add r2 bucket name binding to wrangler.toml for staging, production #578 (comment) unblocks: * `DbDelegationsStorageWithR2` supports deletes * we don't need this to support current `DelegationsStorage`, but probably will before long, but doesn't need to block this PR * optimization where r2 stores all the CIDs with varying multibases but for the same hash only once * safe to remove `DbDelegationsStorage`, `DelegationsV2Row`, `DelegationsV2Tables` --------- Co-authored-by: Irakli Gozalishvili <contact@gozala.io>

🤖 I have created a release *beep* *boop* --- ## [5.0.0](access-api-v4.11.0...access-api-v5.0.0) (2023-03-23) ### ⚠ BREAKING CHANGES * implement new account-based multi-device flow ([#433](#433)) * upgrade capabilities to latest ucanto ([#463](#463)) ### Features * access-api handles provider/add invocations ([#462](#462)) ([46da0df](46da0df)) * access-api serves access/claim invocations ([#456](#456)) ([2ec16e9](2ec16e9)) * access/authorize confirmation email click results in a delegation back to the issuer did:key so that access/claim works ([#460](#460)) ([fc62691](fc62691)) * allow multiple providers ([#595](#595)) ([aba57b3](aba57b3)) * define `access/confirm` handler and use it in ucanto-test-utils registerSpaces + validate-email handler ([#530](#530)) ([a08b513](a08b513)) * handle access/delegate invocations without error ([#427](#427)) ([db01d07](db01d07)) * if POST /validate-email?mode=authorize catches error w/ too big qr code ([#516](#516)) ([ab83b19](ab83b19)) * implement new account-based multi-device flow ([#433](#433)) ([6152e55](6152e55)) * includes proofs chains in the delegated authorization chain ([#467](#467)) ([743a72f](743a72f)) * move access-api delegation bytes out of d1 and into r2 ([#578](#578)) ([3029e4a](3029e4a)) * move validation flow to a Durable Object to make it ⏩ fast ⏩ fast ⏩ fast ⏩ ([#449](#449)) ([3868d97](3868d97)) * provision provider type is now the DID of the w3s service ([#528](#528)) ([4cd6cd9](4cd6cd9)) * space/info will not error for spaces that have had storage provider added via provider/add ([#510](#510)) ([362024f](362024f)) * upgrade capabilities to latest ucanto ([#463](#463)) ([e375ae4](e375ae4)) * upgrade to new ucanto ([#498](#498)) ([790750d](790750d)) * write invocations and receipts into ucan log ([#592](#592)) ([d52a281](d52a281)) ### Bug Fixes * access/delegate checks hasStorageProvider(space) in a way that provider/add allows access/delegate ([#483](#483)) ([1d3d562](1d3d562)) * adjust migration 0005 to keep delegations table but create new used delegations_v2 ([#469](#469)) ([d90825a](d90825a)) * adjust migration 0005 to not do a drop table and instead rename delegations -> delegations_old and create a new delegations ([#468](#468)) ([89f2acd](89f2acd)) * allow injecting email ([#466](#466)) ([b4b0173](b4b0173)) * DbDelegationsStorage#find throws UnexpectedDelegation w/ { row } if failed bytesToDelegations ([#476](#476)) ([660f773](660f773)) * DbProvisionsStorage putMany doesnt error on cid col conflict ([#517](#517)) ([8c6dea8](8c6dea8)) * delegations model tries to handle if row.bytes is Array not Buffer (e.g. cloudflare) ([#478](#478)) ([02c0c28](02c0c28)) ### Miscellaneous Chores * **access-client:** release 11.0.0-rc.0 ([#573](#573)) ([29daa02](29daa02)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release *beep* *boop* --- ## [11.0.0](access-v11.0.0-rc.0...access-v11.0.0) (2023-03-23) ### Features * move access-api delegation bytes out of d1 and into r2 ([#578](#578)) ([3029e4a](3029e4a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: Alan Shaw <alan.shaw@protocol.ai>

Motivation: * part of #578

…nger.toml to use env-specific buckets (#603) Motivation: * #578 (comment) * #578 (comment)

🤖 I have created a release *beep* *boop* --- ## [4.2.0](storacha/w3ui@react-uploads-list-v4.1.0...react-uploads-list-v4.2.0) (2023-11-03) ### Features * re-export Service from `react-keyring` ([storacha#577](storacha/w3ui#577)) ([308816d](storacha/w3ui@308816d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Travis Vachon <travis.vachon@protocol.ai>

🤖 I have created a release *beep* *boop* --- ## [4.2.0](storacha/w3ui@react-uploads-list-v4.1.0...react-uploads-list-v4.2.0) (2023-11-03) ### Features * re-export Service from `react-keyring` ([storacha#577](storacha/w3ui#577)) ([94e8b96](storacha/w3ui@94e8b96)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Travis Vachon <travis.vachon@protocol.ai>

gobengo added 2 commits March 20, 2023 16:05

start delegations store

1e83d67

test a bit

198e32d

gobengo self-assigned this Mar 20, 2023

gobengo changed the title ~~571 delegation bytes out d1~~ feat: move access-api delegation bytes out of d1 and into r2 Mar 20, 2023

gobengo added this to the w3up phase 3 milestone Mar 21, 2023

gobengo added 2 commits March 21, 2023 14:58

start delegations v3 table

963a0d4

DbDelegationsStorageWithR2 reads/writes to r2

e957d94

gobengo force-pushed the 571-delegation-bytes-out-d1 branch from 5ea4459 to e957d94 Compare March 22, 2023 00:18

gobengo added 5 commits March 21, 2023 17:20

add comment explaining v2-only test

83c9c49

add todo

e65dd19

todo

b491acb

fix issues from conflicting wrangler, miniflare, @miniflare/r2 versio…

000c704

…ns across packages in this monorepo

access-api adds @miniflare/r2 as devDependency, fixing tsc

044f76b

gobengo commented Mar 22, 2023

View reviewed changes

packages/access-api/package.json Outdated Show resolved Hide resolved

gobengo commented Mar 22, 2023

View reviewed changes

packages/access-api/package.json Outdated Show resolved Hide resolved

gobengo commented Mar 22, 2023

View reviewed changes

packages/access-api/package.json Outdated Show resolved Hide resolved

rm unused files

eeab7fd

gobengo commented Mar 22, 2023

View reviewed changes

packages/access-api/src/models/delegations.js Outdated Show resolved Hide resolved

gobengo commented Mar 22, 2023

View reviewed changes

packages/access-api/src/models/delegations.js Outdated Show resolved Hide resolved

gobengo commented Mar 22, 2023

View reviewed changes

packages/access-api/src/models/delegations.js Outdated Show resolved Hide resolved

gobengo commented Mar 22, 2023

View reviewed changes

packages/access-api/src/models/delegations.js Outdated Show resolved Hide resolved

gobengo commented Mar 22, 2023

View reviewed changes

packages/access-api/src/models/delegations.js Show resolved Hide resolved

fix depcheck for access-api

a39b780

gobengo mentioned this pull request Mar 22, 2023

chore: remove access-ws and access-client package tests that rely on it #596

Merged

gobengo added 2 commits March 22, 2023 12:22

Merge branch 'main' into 571-delegation-bytes-out-d1

bd42ee4

chore: loosen wrangler/miniflare semvers to range (#597)

568d095

gobengo commented Mar 22, 2023

View reviewed changes

gobengo added 2 commits March 22, 2023 12:59

models only depend on table types that they need

0186d6a

chore: upgrade kysely-d1 and clean up db-related things due to d1 lib…

9fce623

… upgrades (#598) Motivation: * https://github.com/web3-storage/w3protocol/pull/578/files#r1144141609

gobengo commented Mar 22, 2023

View reviewed changes

packages/access-api/wrangler.toml Show resolved Hide resolved

alanshaw approved these changes Mar 22, 2023

View reviewed changes

Gozala requested changes Mar 23, 2023

View reviewed changes

gobengo mentioned this pull request Mar 23, 2023

chore: rename ACCESS_API_R2 binding to DELEGATIONS, and configure wranger.toml to use env-specific buckets #603

Merged

gobengo and others added 10 commits March 22, 2023 18:21

chore: rename ACCESS_API_R2 binding to DELEGATIONS, and configure wra…

ecbe77d

…nger.toml to use env-specific buckets (#603) Motivation: * #578 (comment) * #578 (comment)

Update packages/access-api/src/models/delegations.js

c77d0cd

Co-authored-by: Irakli Gozalishvili <contact@gozala.io>

Update packages/access-api/src/models/delegations.js

37b05c1

Co-authored-by: Irakli Gozalishvili <contact@gozala.io>

Update packages/access-api/test/delegations-storage.test.js

7f616d8

Co-authored-by: Irakli Gozalishvili <contact@gozala.io>

prettier

6fa5390

remove delegations_v2 schema and DbDelegationsStorage (keep newer DbD…

fe26c70

…elegationsStorageWithR2)

assert cid string kind in d1 cid column and r2 key

e379699

improve car parsing/validating in DbDelegationsStorageWithR2 rowToDel…

7d017e7

…egation

env.DELEGATIONS -> env.DELEGATIONS_BUCKET

e2da517

rename r2 buckets to w3up-delegations-{env}-{i}

3261215

gobengo merged commit 4510c9a into main Mar 23, 2023

gobengo deleted the 571-delegation-bytes-out-d1 branch March 23, 2023 07:34

This was referenced Mar 23, 2023

chore(main): release access-api 5.0.0 #588

Merged

chore(main): release access 11.0.0 #605

Merged

it-dag-house mentioned this pull request Mar 23, 2023

chore(main): release access-api 11.0.0-rc.0 #620

Closed

gobengo added a commit that referenced this pull request Apr 11, 2023

fix: DbDelegationsStorageWithR2 saving to desired r2 key (#600)

982035a

Motivation: * part of #578

gobengo added a commit that referenced this pull request Apr 11, 2023

chore: rename ACCESS_API_R2 binding to DELEGATIONS, and configure wra…

3b4c5c4

…nger.toml to use env-specific buckets (#603) Motivation: * #578 (comment) * #578 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: move access-api delegation bytes out of d1 and into r2 #578

feat: move access-api delegation bytes out of d1 and into r2 #578

gobengo commented Mar 20, 2023 •

edited

Loading

gobengo Mar 22, 2023 •

edited

Loading

alanshaw left a comment

alanshaw Mar 22, 2023

Gozala Mar 22, 2023

gobengo Mar 23, 2023

alanshaw Mar 22, 2023

gobengo Mar 22, 2023

gobengo Mar 23, 2023

Gozala left a comment

Gozala Mar 22, 2023

Gozala Mar 22, 2023

gobengo Mar 23, 2023

Gozala Mar 23, 2023

gobengo Mar 23, 2023

Gozala Mar 23, 2023

gobengo Mar 23, 2023

Gozala Mar 23, 2023

gobengo Mar 23, 2023

gobengo commented Mar 23, 2023 •

edited

Loading

feat: move access-api delegation bytes out of d1 and into r2 #578

feat: move access-api delegation bytes out of d1 and into r2 #578

Conversation

gobengo commented Mar 20, 2023 • edited Loading

gobengo Mar 22, 2023 • edited Loading

Choose a reason for hiding this comment

alanshaw 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

Gozala 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

gobengo commented Mar 23, 2023 • edited Loading

gobengo commented Mar 20, 2023 •

edited

Loading

gobengo Mar 22, 2023 •

edited

Loading

gobengo commented Mar 23, 2023 •

edited

Loading