Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow customers to create more than one space #246

Merged
merged 3 commits into from
Oct 26, 2023

Conversation

travis
Copy link
Member

@travis travis commented Oct 19, 2023

this is just a matter of changing how we generate subscription IDs. they are now generated from customer AND consumer, which means they will be unique for a given customer/consumer pair - in other words, a given customer will only be able to provision a given space one time.

this is just a matter of changing how we generate subscription IDs. they are now generated from customer AND consumer, which means they will be unique for a given customer/consumer pair - in other words, a given customer will only be able to provision a given space one time.
@seed-deploy seed-deploy bot temporarily deployed to pr246 October 19, 2023 17:13 Inactive
travis added a commit to storacha/w3up that referenced this pull request Oct 19, 2023
this just updates the test `ProvisionsStorage` to behave the way the production `ProvisionsStorage` does as of storacha/w3infra#246
@seed-deploy
Copy link

seed-deploy bot commented Oct 19, 2023

View stack outputs

@alanshaw
Copy link
Member

Is there no test for this behaviour that needs to be updated?

@travis
Copy link
Member Author

travis commented Oct 19, 2023

yea, surprisingly there isn't - in retrospect I definitely should have added one during the migration from D3 but it looks like I overlooked it!

There is a test in w3up that broke during that migration because we started restricting users to one space per user, which I skipped because there wasn't an easy way to get it working and we were already talking about loosening the restriction - I've re-enabled that in a separate PR:

storacha/w3up#989

I do think it would be a good idea to have a test that verifies a user can create multiple spaces - thinking through that I think the original test slipped through the cracks because most of the service-level tests live in w3up, but this is explicitly a behavior we'd like implementors to be able to control with their ProvisionsStorage implementation - I think the right test for this lives at the ProvisionsStorage level, so I'll get that into this PR asap.

@seed-deploy seed-deploy bot temporarily deployed to pr246 October 19, 2023 23:36 Inactive
@travis
Copy link
Member Author

travis commented Oct 19, 2023

ok - updated with an integration test!

* by deriving subscription ID from customer AND consumer) in the future
* or by other providers for flexibility.
* uses a CID generated from `customer` and `consumer` which ensures each customer
* will get at most one subscription per space.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'll also allow existing spaces that have already been provisioned to be provisioned again...can we fix/avoid?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh HMMM good point, maybe this should just be generated from "consumer" so that a given space can only ever be provisioned once... will make that tweak and add a test!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh it turns out we already check for this in provisioning! I would like to leave the door open for a "multi-payer" space in the future so I think we should leave as currently implemented!

@alanshaw pointed out that as implemented, a space could be registered by more than one user, which would make billing pretty challenging.

I tried to write a test for this, but w3up-client and access-client go to some lengths to stop a user from registering another user's space so this is a deeper rabbithole than I'd like to go down for now.
@seed-deploy seed-deploy bot temporarily deployed to pr246 October 23, 2023 20:39 Inactive
@gobengo
Copy link
Contributor

gobengo commented Oct 23, 2023

@travis btw I tested w3infra from your commit at aafebb2 locally and confirmed that it resolves storacha/w3cli#112

I'll test again once deployed

travis added a commit to storacha/w3up that referenced this pull request Oct 24, 2023
this just updates the test `ProvisionsStorage` to behave the way the
production `ProvisionsStorage` does as of
storacha/w3infra#246
@travis travis merged commit fe3e3c6 into main Oct 26, 2023
3 checks passed
@travis travis deleted the feat/multiple-spaces branch October 26, 2023 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants