Add an endpoint to download a file #818
Conversation
| @@ -31,3 +31,21 @@ export async function putObjectStream(bucket: string, key: string, body: Readabl | |||
| fileSize, | |||
| } | |||
| } | |||
|
|
|||
| export async function getObjectStream(bucket: string, key: string, range?: { start: number; end: number }) { | |||
| @@ -31,3 +31,21 @@ export async function putObjectStream(bucket: string, key: string, body: Readabl | |||
| fileSize, | |||
| } | |||
| } | |||
|
|
|||
| export async function getObjectStream(bucket: string, key: string, range?: { start: number; end: number }) { | |||
| const client = await getS3Client() | |||
There was a problem hiding this comment.
Do we need to create a new S3 Client for every S3 request?
There was a problem hiding this comment.
Our S3 credentials expire on some platforms relatively frequently. Tracking whether the current S3 client has expired and replacing it seems too complex for a function that's likely to only get called a few hundred - thousand times a day.
| export const getDownloadFileSchema = z.object({ | ||
| params: z.object({ | ||
| modelId: z.string({ | ||
| required_error: 'Must specify model id as param', |
There was a problem hiding this comment.
The standard Zod error here would be sufficient?
| res.set('Content-Type', file.mime) | ||
| res.set('Cache-Control', 'public, max-age=604800, immutable') | ||
|
|
||
| if (req.headers.range) { |
There was a problem hiding this comment.
Do this first before logic for response?
There was a problem hiding this comment.
Range requests will include the same headers (Content-Type, Cache-Control and Content-Disposition) seen above. The idea behind placing it here is that when we do support it you can just write the implementation in here and not need to change anything else.
I could move the error up (rule: do error checking first), but it feels like a waste for now.
| } | ||
|
|
||
| // The AWS library doesn't seem to properly type 'Body' as being pipeable? | ||
| ;(stream.Body as any).pipe(res) |
There was a problem hiding this comment.
Can you raise this issue on their repo?
There was a problem hiding this comment.
Existing issue here: aws/aws-sdk-js-v3#4720
Also narrowed type to stream.Readable from any. Issue seems to be that the AWS client is isomorphic, but you only get streams on Node so they're loosely typed.
|
|
||
| // The AWS library doesn't seem to properly type 'Body' as being pipeable? | ||
| ;(stream.Body as any).pipe(res) | ||
| }, |
There was a problem hiding this comment.
Am I missing something, where is the method's return statement?
There was a problem hiding this comment.
We don't need to return anything. The pipe will close the response stream when the Body is done being streamed to the user.
| @@ -37,6 +37,18 @@ export async function uploadFile(user: UserDoc, modelId: string, name: string, m | |||
| return file | |||
| } | |||
|
|
|||
| export async function downloadFile(user: UserDoc, fileId: string, range?: { start: number; end: number }) { | |||
No description provided.