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.

Sign up for GitHub

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

Jump to bottom

feat: implement consumer/get and subscription/get invocation handlers #220

Merged
merged 14 commits into from
Aug 31, 2023
Merged

feat: implement consumer/get and subscription/get invocation handlers #220

merged 14 commits into from
Aug 31, 2023

Conversation

travis
Copy link
Member

@travis travis commented Aug 18, 2023 •
edited
Loading

consumer/get required wiring up the space metrics table in the upload-api stack to get space allocation metrics, which we call "allocation" in w3admin. Currently just returning 1 billion as the "total" space for all consumers since we don't currently check anyway, but we should figure out where this lives when we tackle billing in the near future.

Also implement getStorageProviders per storacha/w3up#862

Also add subscriptions to the customer/get response, per storacha/w3up#863

@travis
feat: implement consumer/get invocation handler
4566698 
this required wiring up the space metrics table in the upload-api stack, but works pretty well!

currently just returning 1 billion as the "total" space for all consumers since we don't currently check anyway
@seed-deploy seed-deploy bot temporarily deployed to pr220 August 18, 2023 19:19 Inactive
@seed-deploy
Copy link

seed-deploy bot commented Aug 18, 2023

View stack outputs

@travis
feat: implement getSubscription method of ProvisionStore
7ab75ab 
this should enable `subscription/get` invocation to work as one would expect
@travis
feat: add subscriptions to customer record
de93ce2 
per w3up change here: storacha/w3up#863
@travis travis changed the title feat: implement consumer/get invocation handler feat: implement consumer/get and subscription/get invocation handlers Aug 18, 2023
@travis travis marked this pull request as draft August 18, 2023 23:13
@travis travis marked this pull request as ready for review August 18, 2023 23:13
@seed-deploy seed-deploy bot temporarily deployed to pr220 August 22, 2023 21:03 Inactive
@travis
feat: implement missing functions
7c49e0d 
1) bubble getStorageProviders up to ProvisionStore
2) implement get on SubscriptionTable
3) implement getBySubscription on ConsumerTable
@seed-deploy seed-deploy bot temporarily deployed to pr220 August 22, 2023 21:34 Inactive
@seed-deploy seed-deploy bot temporarily deployed to pr220 August 23, 2023 20:21 Inactive
@seed-deploy seed-deploy bot temporarily deployed to pr220 August 28, 2023 19:29 Inactive
@travis travis requested a review from alanshaw August 28, 2023 20:05
vasco-santos
vasco-santos previously requested changes Aug 30, 2023
View reviewed changes
Copy link
Contributor

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

This looks good, just pointed out some small things. More critical would be to make sure we use constants for metrics names to avoid breaking things on changes

upload-api/types.ts Outdated Show resolved Hide resolved
upload-api/tables/space-metrics.js Outdated Show resolved Hide resolved
upload-api/tables/subscription.js Outdated Show resolved Hide resolved
upload-api/tables/consumer.js Outdated Show resolved Hide resolved
upload-api/tables/consumer.js Show resolved Hide resolved
@travis @vasco-santos
Apply suggestions from code review
47ac493 
Co-authored-by: Vasco Santos <santos.vasco10@gmail.com>
@seed-deploy seed-deploy bot temporarily deployed to pr220 August 31, 2023 03:45 Inactive
@seed-deploy seed-deploy bot temporarily deployed to pr220 August 31, 2023 03:54 Inactive
@seed-deploy seed-deploy bot temporarily deployed to pr220 August 31, 2023 04:02 Inactive
@seed-deploy seed-deploy bot temporarily deployed to pr220 August 31, 2023 04:12 Inactive
vasco-santos
vasco-santos approved these changes Aug 31, 2023
View reviewed changes
Copy link
Contributor

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

LGTM

@travis travis dismissed vasco-santos’s stale review August 31, 2023 22:42

subsequent approval further downthread

@travis travis merged commit e1c0507 into main Aug 31, 2023
1 check passed
@travis travis deleted the feat/get-consumer branch August 31, 2023 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vasco-santos vasco-santos vasco-santos approved these changes

@olizilla olizilla Awaiting requested review from olizilla

@Gozala Gozala Awaiting requested review from Gozala

@alanshaw alanshaw Awaiting requested review from alanshaw

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@travis @alanshaw @vasco-santos