feat!: fold access-api into upload-api - #194
Conversation
View stack outputs
|
Gozala
left a comment
Gozala
left a comment
There was a problem hiding this comment.
As a direct port of what we have in d1 this looks all right. That said, I think taking bit more time to avoid another migration down the line might be in our benefit, from that perspective I have following notes:
-
We need two tables as opposed to currently 1 provisions table, which
That way when you first login web3.storage can delegate an account a subscription that account can later add a consumer (space) to.
- Tracks subscriptions as in provider <-> customer relationship which is where provider could impose any constraints it wishes on the customer.
- Tracks subscription consumer provider <-> consumer <-> subscription relationship.
-
We need to support different actors to perform following queries
- provider:
- find my customers (accounts that have subscriptions)
- find my consumers (spaces subscribed)
- cancel subscription
- block customer
- stats per customer / consumer / subscription
- space
- what are my providers / subscriptions
- unsubscribe
- stats / limits
- customer (account)
- cancel subscription
- what are my subscriptions
- stats per subscriptions
- provider:
-
we need to make provider capable of imposing whatever constraints it wants on accounts.
- I propose accomplishing this via additional
orderfield with uniqueness constraint. This way provider can impose all the constraints it wants by encoding them into the field generation.- e.g. web3.storage can derive order from the account email address.
- nft.storage could choose alternative that would fit it's goals.
- I propose accomplishing this via additional
This is a first step toward storacha/w3up#711 as documented in https://github.com/web3-storage/w3up/pull/744/files?short_path=4e012d9#diff-4e012d97ece45b455806fabbe1f93ce11af56213b9137463a427e1c5d4d8bda6 This is an implementation spike of storing delegations in DynamoDB rather than D1. Because we already use DynamoDB in the upload-api, it made sense to reuse the configuration and testing infrastructure we've already built there. I copied a number of interfaces from https://github.com/web3-storage/w3up/tree/main/packages/access-api to get this done, and will migrate back to them if that ends up making sense. As this implies, the path forward for this is somewhat unclear and I need feedback and consensus from the team to keep this moving. We have a couple options to move forward: 1) Port the code in this PR over to the `access-api` repository and run it inside a cloudflare worker. This will require us to port some of the configuration and testing utilities in this repository to that repository and will leave us with a situation where compute in Cloudflare is reading and writing to DynamoDB in AWS. It will also require us to change the S3 logic in this PR to operate against R2, but that may Just Work thanks to theoretical API compatibility. This will also result in AWS-related logic being split between this repository and w3up, and we will probably want to extract common utilities to a separate package at some point in the future. 2) Port the delegations-related service endpoints in `w3up/access-api` over to this repository and get them running in an AWS Lambda. In this scenario we could either use S3 as implemented in this PR or continue to store delegations in R2. This option would move us in the direction of abstracting all operational details out of the w3up repository, following the pattern already used by w3up's `upload-api` package, which is imported by `upload-api` in this package. In this option I think it probably makes sense to migrate existing delegations from R2 to S3, but could be wrong on that front. My conversations with @Bengo suggested that the team may be mostly aligned behind option (2), especially because it seems like it moves us in the direction of isolating operational details of our services to this repository and turning the w3ui `*-api` packages into abstract implementations of the service providers we implement. Thoughts?
implement a test that represents my current best understanding of how provisions should work for now.
feedback from w3infra PR: storacha/w3infra#194
@Gozala doesn't like them and made a good argument against this here: storacha/w3infra#194 (comment)
123137d to
db9198c
Compare
- add necessary context to run new providers - add validate-email GET and POST handlers as a new Lambda
travis
changed the title
access-api into upload-api
configured and paid for a month of MailSlurp so we can take it for a test drive
reviewed this PR and made a bunch of updates
they weren't being added because Lambda is a special snowflake
vasco-santos
left a comment
vasco-santos
left a comment
There was a problem hiding this comment.
@travis left new round of feedback. We are close, most comments relate to prepare ourselves better for the upcoming follow ups from this PR that were identified
Stack outputs updated
|
I'll reintroduce these in a separate branch
vasco-santos
left a comment
vasco-santos
left a comment
There was a problem hiding this comment.
This LGTM! Thanks for the great work here @travis
As we talked, let's get the migration code here and tested in staging only next week. So that we don't have main branch blocked until then
To run: ``` # first ensure you are logged in to wrangler with `wrangler login` STAGE=pr194 npm run d1-dynamo-migration ``` --------- Co-authored-by: Irakli Gozalishvili <contact@gozala.io>
grab data from D1 and use it to verify that all the bits got moved correctly during the course of this I found a bug in the migration script - whew! thanks for the suggestion @vasco-santos also add a simple utility to list the emails of folks who have registered for w3up
emails don't seem to be making it through within 30 seconds
| * Abstraction layer to handle operations on Store Table. | ||
| * | ||
| * @param {string} region | ||
| * @param {string} tableName | ||
| * @param {object} [options] | ||
| * @param {string} [options.endpoint] | ||
| */ | ||
| export function createSubscriptionTable (region, tableName, options = {}) { | ||
| const dynamoDb = new DynamoDBClient({ | ||
| region, | ||
| endpoint: options.endpoint, | ||
| }) | ||
|
|
||
| return useSubscriptionTable(dynamoDb, tableName) | ||
| } | ||
|
|
||
| /** | ||
| * @param {DynamoDBClient} dynamoDb | ||
| * @param {string} tableName | ||
| * @returns {SubscriptionTable} | ||
| */ | ||
| export function useSubscriptionTable (dynamoDb, tableName) { |
There was a problem hiding this comment.
I do not want to block this PR, but I still dislike amount of incidental complexity we have for every single table which I have described a solution for in another PR, that I think applies here just as well
storacha/w3filecoin-infra#26 (comment)
Perhaps we could remove some of this complexity later as a cleanup.
There was a problem hiding this comment.
I hear you on the incidental complexity, but to be honest this boilerplate doesn't bother me much - having one initializer per table that takes a generic client and another that takes a set of options to set up a client in a default way feels pretty normal and good to me.
That said I'm definitely not opposed to putting together a cleaner/easier-to-use abstraction for this. From my read of the comment thread you linked, @vasco-santos identified some additional complexity that isn't accounted for by the solution you propose.
If you're up for it, I think it would be a good idea for you to do a quick spike (including integrating it into tests and other places where we use these initializers - migration scripts, etc) of what you're thinking on a single table and then have the three of us do a (high bandwidth maybe?) review of the solution to make sure it fits all our needs and is strictly better than what we have right now. I'd be more than happy to take that spike and run with it, updating our usage across the board. I'd also be down to do the spike, but would definitely need to sync with you and Vasco to summarize and dig into the thinking on this thread so far.
All that said, I appreciate you looking out for these abstractions! I'm adding yet another table so I do think investing in this will help us keep things readable and usable.
3 participants
Get
@web3-storage/access-apihandlers running in theupload-apiservice.This introduces two new DynamoDB tables to store "delegations" and "provisions". NB: I have a related PR in #200 that uses "consumers" and "subscriptions" tables rather than "provisions".
This also adds a new Lambda to handle the email verification flow - I've brought some HTML utilities over from
@web3-storage/access-apifor this purpose and wired the handlers up to the same path they were at in the old service/validate-email.This is the part 2 of a larger project (started in storacha/w3up#790) to integrate the access service into the w3infra upload service, which will let us move operational details out of w3up, retire D1, consolidate our operations around AWS Lambdas, deprecate now-unused functionality like the voucher flows and generally streamline our UCAN handlers and the infrastructure that supports them.
TODO
access-apiintoupload-api#194 (review)ProvisionStoragein terms of subscription and consumer tables #200access-apiintoupload-api#194 (comment))@web3-storage/upload-apiis released