Slim down API, focus on happy path #1
Conversation
example/convex/example.ts
Outdated
| onUpload: async (ctx, bucket, key) => { | ||
| // ...do something with the key | ||
| // This technically runs in the `syncMetadata` mutation, as the upload | ||
| // is performed from the client side. Will run if using the `useUploadFile` | ||
| // hook, or if `syncMetadata` function is called directly. Runs after the | ||
| // `checkUpload` callback. | ||
| await ctx.db.insert("images", { | ||
| bucket, | ||
| key, |
There was a problem hiding this comment.
how would you associate this with a message? I guess that would be more like using the useUploadFile callback and then taking the key and chaining another mutation? or a future version could pass args to the useUploadFile or something..
There was a problem hiding this comment.
I may not be understanding your question here, but the callback provides the object key, which you can then associate with any other piece of data (such as a message). Is that what you meant?
There was a problem hiding this comment.
Yeah, my question here is how you'd know which message to associate it with, since the callback only knows the bucket & key, not any context about why it was uploaded. The older approach was to provide the URL where they could pull the query arg. With this, it seems they'd need to drive it from the client (assuming the client didn't go away in the meantime).
There was a problem hiding this comment.
Added a note indicating to do stuff like that on the client with the key returned by the useUploadFile function. Notes both here and in the client example.
example/convex/example.ts
Outdated
| // the r2 component's `deleteObject` function. | ||
| const image = await ctx.db | ||
| .query("images") | ||
| .filter((q) => q.eq(q.field("key"), key)) |
There was a problem hiding this comment.
let's make the example use an index to encourage folks to use them
| args: {}, | ||
| handler: async (ctx) => { | ||
| const images = await ctx.db.query("images").take(10); | ||
| const images = await ctx.db.query("images").collect(); |
There was a problem hiding this comment.
maybe this could show off listing / paginating from the component's data? maybe looking up image by key, or doing the lookup the other way
There was a problem hiding this comment.
I originally had it metadata driven, but went the other way since metadata driven is less likely to be the real world scenario. I can take it that way for the sake of the example, though.
There was a problem hiding this comment.
Yeah this approach is fine, there just isn't an example of listing or fetching metadata to exercise that code. Doesn't need to block initial release
There was a problem hiding this comment.
Example now includes both approaches
| await ctx.scheduler.runAfter(0, this.component.lib.deleteObject, { | ||
| key: args.key, | ||
| ...this.r2Config, | ||
| }); |
There was a problem hiding this comment.
I'd like this to be a mutation call to the component so the metadata is deleted transactionally, with the scheduler used just for the action-ey part of sending the command. So either call deleteMetadata with an extra param to also delete the object, or deleteObject doesn't delete metadata and is scheduled separately from the thick client
There was a problem hiding this comment.
Just exposing deleteObject, optimistically deletes metadata and uses the action retrier for a best effort at deleting the underlying object. I suspect this is good enough for now given the metadata in Convex isn't necessarily up to date with R2 anyway, but let me know if you want stronger guarantees.
There was a problem hiding this comment.
sounds great. i'd rather not show the metadata in Convex but have an orphan in R2 than the opposite. It's already possible to have R2 orphans if you upload to the URL but the client never calls up to sync the metadata
|
@ianmacartney this is ready for re-review. |
Focus on simplifying R2 uploads, serving, and providing reactive metadata.
useUploadFilehook to simplify signed url uploads that don't require a lot of control