-
Notifications
You must be signed in to change notification settings - Fork 75
feat(vercel): add Storage resource for managing Vercel storage #737
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
base: main
Are you sure you want to change the base?
Conversation
alchemy/src/vercel/storage.ts
Outdated
/** | ||
* The storage store information | ||
*/ | ||
store: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to pull this out into its own interface since it's a nested object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or should we just flatten these properties into the Storage
interface?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i ended up merging them in the Storage
interface, however, it does feel "messy", because we are merging the API response to it and some properties can overlap... but looking at other resources, they are all doing that (aka merging back to the main interface), so i'm just following it too
alchemy/src/vercel/storage.ts
Outdated
/** | ||
* The team ID that the storage belongs to | ||
*/ | ||
teamId: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we'll eventually have a Team
resource? If yes, then this should be team: string
so that it can be later team: string | Team
which is our convention for supporting both identifiers and references to alchemy resources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a Team
entity on Vercel, not sure if we'll ended up using (depends on which resources are migrated to alchemy)
i added a basic type for VercelTeam
in vercel.types.ts
alchemy/src/vercel/storage.ts
Outdated
/** | ||
* The region where the storage will be deployed | ||
*/ | ||
region: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a union type of allowed regions we can add to make this easier?
I see: https://vercel.com/docs/edge-network/regions
type VercelRegion =
| "cpt1" // Cape Town, South Africa
| "cle1" // Cleveland, USA
| "dxb1" // Dubai, UAE
| "dub1" // Dublin, Ireland
| "fra1" // Frankfurt, Germany
| "hkg1" // Hong Kong
| "lhr1" // London, UK
| "bom1" // Mumbai, India
| "kix1" // Osaka, Japan
| "cdg1" // Paris, France
| "pdx1" // Portland, USA
| "sfo1" // San Francisco, USA
| "gru1" // São Paulo, Brazil
| "icn1" // Seoul, South Korea
| "sin1" // Singapore
| "arn1" // Stockholm, Sweden
| "syd1" // Sydney, Australia
| "hnd1" // Tokyo, Japan
| "iad1"; // Washington, D.C., USA
| (string & {}) // special rune to maintain auto-suggestions without closing the type to new regions or ones we missed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great point, i was looking from a local experiment only. fixed it.
alchemy/src/vercel/storage.ts
Outdated
/** | ||
* The desired name for the storage | ||
*/ | ||
name: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another convention we have is to leave this as optional name?: string
and "@default id`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i checked upstash
and vercel/project
.... what you suggested is not happening! so i just followed their examples...
anyway, i added the case as you suggested (fallback to id
) in the Storage
class
alchemy/src/vercel/storage.ts
Outdated
async function createStorage(api: VercelApi, props: StorageProps) { | ||
const response = await api.post(`/storage/stores/${props.type}?teamId=${props.teamId}`, { | ||
name: props.name, | ||
region: props.region, | ||
}); | ||
return response.json() as Promise<{ store: Storage["store"] }>; | ||
} | ||
|
||
/** | ||
* Delete a storage instance | ||
*/ | ||
async function deleteStorage(api: VercelApi, output: Storage) { | ||
await api.delete(`/storage/stores/${output.id}/connections?teamId=${output.teamId}`); | ||
await api.delete(`/storage/stores/${output.type}/${output.id}?teamId=${output.teamId}`); | ||
} No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be checking status code and throwing errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the api.*
client is returning a custom error if fetch
is not response.ok
i ended up adding neverthrow
on these functions, but there's no custom error, just bubbling up the returned error
This is looking great. Should add some tests in alchemy/test/vercel/storage.test.ts and follow the same practices as the |
Hi @oieduardorabelo I tried pushing some changes to your Pr bur I don't have permission to push to your fork. Usually I do, not sure if you disabled that permission. I opened this PR with the changes: #822. I'd prefer to continue with your PR, but LMK if you'd prefer I took over and got this merged. |
f1acd44
to
195ee2e
Compare
@sam-goodwin man, i have no idea why you don't have permission 😂 i'm back today + i'll try to fix this permission issue + rebase your branch into this pr |
e3dcdf6
to
dc13514
Compare
a3a4abd
to
1eeb802
Compare
Thanks for the support via Discord!
Support for Vercel's
blob
storage onlyHappy to contribute more if there is demand
final API: