From 490c4c75b1d36699db2d20368beb41b91936a385 Mon Sep 17 00:00:00 2001 From: ARADDCC012 <110473008+ARADDCC012@users.noreply.github.com> Date: Tue, 12 Nov 2024 18:07:49 +0000 Subject: [PATCH 1/7] Refactored release export to stream files separately --- backend/src/services/mirroredModel.ts | 105 ++++++++++++++------------ 1 file changed, 57 insertions(+), 48 deletions(-) diff --git a/backend/src/services/mirroredModel.ts b/backend/src/services/mirroredModel.ts index 613526ab3..50fff543c 100644 --- a/backend/src/services/mirroredModel.ts +++ b/backend/src/services/mirroredModel.ts @@ -58,17 +58,7 @@ export async function exportModel( const s3Stream = new PassThrough() zip.pipe(s3Stream) - if (config.modelMirror.export.kmsSignature.enabled) { - log.debug({ modelId, semvers }, 'Using signatures. Uploading to temporary S3 location first.') - uploadToTemporaryS3Location(modelId, semvers, s3Stream).then(() => - copyToExportBucketWithSignatures(modelId, semvers, mirroredModelId, user.dn).catch((error) => - log.error({ modelId, semvers, error }, 'Failed to upload export to export location with signatures'), - ), - ) - } else { - log.debug({ modelId, semvers }, 'Signatures not enabled. Uploading to export S3 location.') - uploadToExportS3Location(modelId, semvers, s3Stream, { modelId, mirroredModelId }) - } + uploadToS3(`${modelId}.zip`, s3Stream, user.dn, { modelId, semvers }, { modelId, mirroredModelId }) try { await addModelCardRevisionsToZip(user, model, zip) @@ -77,7 +67,7 @@ export async function exportModel( } try { if (semvers && semvers.length > 0) { - await addReleasesToZip(user, model, semvers, zip) + await addReleasesToZip(user, model, semvers, zip) // Adds releases to zip file } } catch (error) { throw InternalError('Error when adding the release(s) to the zip file.', { error }) @@ -167,50 +157,68 @@ function parseModelCard(modelCardJson: string, mirroredModelId: string, sourceMo return { modelCard } } +async function uploadToS3( + fileName: string, + stream: Readable, + exporter: string, + logData: Record, + metadata?: Record, +) { + if (config.modelMirror.export.kmsSignature.enabled) { + log.debug(logData, 'Using signatures. Uploading to temporary S3 location first.') + uploadToTemporaryS3Location(fileName, stream, logData).then(() => + copyToExportBucketWithSignatures(fileName, exporter, logData, metadata).catch((error) => + log.error({ error, ...logData }, 'Failed to upload export to export location with signatures'), + ), + ) + } else { + log.debug(logData, 'Signatures not enabled. Uploading to export S3 location.') + uploadToExportS3Location(fileName, stream, logData, metadata) + } +} + async function copyToExportBucketWithSignatures( - modelId: string, - semvers: string[] | undefined, - mirroredModelId: string, + fileName: string, exporter: string, + logData: Record, + metadata?: Record, ) { let signatures = {} - log.debug({ modelId, semvers }, 'Getting stream from S3 to generate signatures.') - const streamForDigest = await getObjectFromTemporaryS3Location(modelId, semvers) + log.debug(logData, 'Getting stream from S3 to generate signatures.') + const streamForDigest = await getObjectFromTemporaryS3Location(fileName, logData) const messageDigest = await generateDigest(streamForDigest) - log.debug({ modelId, semvers }, 'Generating signatures.') + log.debug(logData, 'Generating signatures.') try { signatures = await sign(messageDigest) } catch (e) { - log.error({ modelId }, 'Error generating signature for export.') + log.error(logData, 'Error generating signature for export.') throw e } - log.debug({ modelId, semvers }, 'Successfully generated signatures') - log.debug({ modelId, semvers }, 'Getting stream from S3 to upload to export location.') - const streamToCopy = await getObjectFromTemporaryS3Location(modelId, semvers) - await uploadToExportS3Location(modelId, semvers, streamToCopy, { - modelId, - mirroredModelId, + log.debug(logData, 'Successfully generated signatures') + log.debug(logData, 'Getting stream from S3 to upload to export location.') + const streamToCopy = await getObjectFromTemporaryS3Location(fileName, logData) + await uploadToExportS3Location(fileName, streamToCopy, logData, { exporter, ...signatures, + ...metadata, }) } async function uploadToTemporaryS3Location( - modelId: string, - semvers: string[] | undefined, + fileName: string, stream: Readable, + logData: Record, metadata?: Record, ) { const bucket = config.s3.buckets.uploads - const object = `exportQueue/${modelId}.zip` + const object = `exportQueue/${fileName}` try { await putObjectStream(bucket, object, stream, metadata) log.debug( { bucket, object, - modelId, - semvers, + ...logData, }, 'Successfully uploaded export to temporary S3 location.', ) @@ -219,26 +227,24 @@ async function uploadToTemporaryS3Location( { bucket, object, - modelId, - semvers, error, + ...logData, }, 'Failed to export to temporary S3 location.', ) } } -async function getObjectFromTemporaryS3Location(modelId: string, semvers: string[] | undefined) { +async function getObjectFromTemporaryS3Location(fileName: string, logData: Record) { const bucket = config.s3.buckets.uploads - const object = `exportQueue/${modelId}.zip` + const object = `exportQueue/${fileName}` try { const stream = (await getObjectStream(bucket, object)).Body as Readable log.debug( { bucket, object, - modelId, - semvers, + ...logData, }, 'Successfully retrieved stream from temporary S3 location.', ) @@ -248,9 +254,8 @@ async function getObjectFromTemporaryS3Location(modelId: string, semvers: string { bucket, object, - modelId, - semvers, error, + ...logData, }, 'Failed to retrieve stream from temporary S3 location.', ) @@ -259,21 +264,19 @@ async function getObjectFromTemporaryS3Location(modelId: string, semvers: string } async function uploadToExportS3Location( - modelId: string, - semvers: string[] | undefined, + object: string, stream: Readable, + logData: Record, metadata?: Record, ) { const bucket = config.modelMirror.export.bucket - const object = `${modelId}.zip` try { await putObjectStream(bucket, object, stream, metadata) log.debug( { bucket, object, - modelId, - semvers, + ...logData, }, 'Successfully uploaded export to export S3 location.', ) @@ -282,9 +285,8 @@ async function uploadToExportS3Location( { bucket, object, - modelId, - semvers, error, + ...logData, }, 'Failed to export to export S3 location.', ) @@ -323,9 +325,16 @@ async function addReleaseToZip(user: UserInterface, model: ModelDoc, release: Re zip.append(JSON.stringify(release.toJSON()), { name: `${baseUri}/releaseDocument.json` }) for (const file of files) { zip.append(JSON.stringify(file.toJSON()), { name: `${baseUri}/files/${file._id}/fileDocument.json` }) - zip.append((await downloadFile(user, file._id)).Body as stream.Readable, { - name: `${baseUri}/files/${file._id}/fileContent`, - }) + uploadToS3( + `${baseUri}/files/${file._id}/fileContent`, + (await downloadFile(user, file._id)).Body as stream.Readable, + user.dn, + { + modelId: model.id, + releaseId: release.id, + fileId: file.id, + }, + ) } } catch (error: any) { throw InternalError('Error when generating the zip file.', { error }) From 80c42fc96232efd85e39186ef4d6983dbfb8101e Mon Sep 17 00:00:00 2001 From: ARADDCC012 <110473008+ARADDCC012@users.noreply.github.com> Date: Mon, 18 Nov 2024 21:30:18 +0000 Subject: [PATCH 2/7] Added awaits for s3 uploading --- backend/src/services/mirroredModel.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/src/services/mirroredModel.ts b/backend/src/services/mirroredModel.ts index 3c769a2c3..4d98f34a9 100644 --- a/backend/src/services/mirroredModel.ts +++ b/backend/src/services/mirroredModel.ts @@ -59,7 +59,7 @@ export async function exportModel( const s3Stream = new PassThrough() zip.pipe(s3Stream) - uploadToS3(`${modelId}.zip`, s3Stream, user.dn, { modelId, semvers }, { modelId, mirroredModelId }) + await uploadToS3(`${modelId}.zip`, s3Stream, user.dn, { modelId, semvers }, { modelId, mirroredModelId }) try { await addModelCardRevisionsToZip(user, model, zip) @@ -330,7 +330,7 @@ async function addReleaseToZip(user: UserInterface, model: ModelDoc, release: Re zip.append(JSON.stringify(release.toJSON()), { name: `${baseUri}/releaseDocument.json` }) for (const file of files) { zip.append(JSON.stringify(file.toJSON()), { name: `${baseUri}/files/${file._id}/fileDocument.json` }) - uploadToS3( + await uploadToS3( `${baseUri}/files/${file._id}/fileContent`, (await downloadFile(user, file._id)).Body as stream.Readable, user.dn, From 3409d4598394fe82716785e570ba20c81c9696c1 Mon Sep 17 00:00:00 2001 From: ARADDCC012 <110473008+ARADDCC012@users.noreply.github.com> Date: Mon, 18 Nov 2024 21:32:35 +0000 Subject: [PATCH 3/7] Removed comment --- backend/src/services/mirroredModel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/services/mirroredModel.ts b/backend/src/services/mirroredModel.ts index 4d98f34a9..1eb16ea8c 100644 --- a/backend/src/services/mirroredModel.ts +++ b/backend/src/services/mirroredModel.ts @@ -68,7 +68,7 @@ export async function exportModel( } try { if (semvers && semvers.length > 0) { - await addReleasesToZip(user, model, semvers, zip) // Adds releases to zip file + await addReleasesToZip(user, model, semvers, zip) } } catch (error) { throw InternalError('Error when adding the release(s) to the zip file.', { error }) From dcd3b636bf1b9ee58eeb484c83b7df5b0c8e1466 Mon Sep 17 00:00:00 2001 From: ARADDCC012 <110473008+ARADDCC012@users.noreply.github.com> Date: Mon, 18 Nov 2024 21:53:36 +0000 Subject: [PATCH 4/7] Updated mirroredModel export tests --- backend/test/services/mirroredModel.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/test/services/mirroredModel.spec.ts b/backend/test/services/mirroredModel.spec.ts index 753b57472..1ed2d11f1 100644 --- a/backend/test/services/mirroredModel.spec.ts +++ b/backend/test/services/mirroredModel.spec.ts @@ -216,6 +216,7 @@ describe('services > mirroredModel', () => { test('exportModel > successful export if no files exist', async () => { releaseMocks.getAllFileIds.mockResolvedValueOnce([]) + fileMocks.getFilesByIds.mockResolvedValueOnce([]) await exportModel({} as UserInterface, 'modelId', true, ['1.2.3', '1.2.4']) // Allow for completion of asynchronous content await new Promise((r) => setTimeout(r)) @@ -314,7 +315,7 @@ describe('services > mirroredModel', () => { test('exportModel > export uploaded to S3 for model cards and releases', async () => { await exportModel({} as UserInterface, 'modelId', true, ['1.2.3', '3.2.1']) - expect(s3Mocks.putObjectStream).toBeCalledTimes(2) + expect(s3Mocks.putObjectStream).toBeCalledTimes(3) }) test('exportModel > unable to upload to tmp S3 location', async () => { From 25862c65f62f7bd1ccb809642aaffc065e7d0ae5 Mon Sep 17 00:00:00 2001 From: JR40159 <126243293+JR40159@users.noreply.github.com> Date: Tue, 19 Nov 2024 12:18:27 +0000 Subject: [PATCH 5/7] Update swagger --- backend/src/routes/v2/model/postRequestExport.ts | 4 ++-- .../src/routes/v2/review/postAccessRequestReviewResponse.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/src/routes/v2/model/postRequestExport.ts b/backend/src/routes/v2/model/postRequestExport.ts index 93dcc055b..9e71f48cd 100644 --- a/backend/src/routes/v2/model/postRequestExport.ts +++ b/backend/src/routes/v2/model/postRequestExport.ts @@ -10,7 +10,7 @@ import { parse } from '../../../utils/validate.js' export const postRequestExportToS3Schema = z.object({ params: z.object({ - modelId: z.string(), + modelId: z.string().openapi({ example: 'yolo-v4-abcdef' }), }), body: z.object({ disclaimerAgreement: z.boolean(), @@ -19,7 +19,7 @@ export const postRequestExportToS3Schema = z.object({ registerPath({ method: 'post', - path: '/api/v2/model/:modelId/export/s3', + path: '/api/v2/model/{modelId}/export/s3', tags: ['model', 'mirror'], description: 'Request for all current model card revisions to be exported to S3 as a Zip file. Can also include releases specified by semver in the body.', diff --git a/backend/src/routes/v2/review/postAccessRequestReviewResponse.ts b/backend/src/routes/v2/review/postAccessRequestReviewResponse.ts index 69f0434a6..30eda1a0f 100644 --- a/backend/src/routes/v2/review/postAccessRequestReviewResponse.ts +++ b/backend/src/routes/v2/review/postAccessRequestReviewResponse.ts @@ -38,8 +38,8 @@ export const postAccessRequestReviewResponseSchema = z.object({ registerPath({ method: 'post', - path: '/api/v2/model/{modelId}/access-request/{acessRequestId}/review', - tags: ['accessRequest', 'review'], + path: '/api/v2/model/{modelId}/access-request/{accessRequestId}/review', + tags: ['access-request', 'review'], description: 'Send a review for an access request.', schema: postAccessRequestReviewResponseSchema, responses: { From f3a1a122d2f437b5df0996ed9f7ec09bd261f6ce Mon Sep 17 00:00:00 2001 From: ARADDCC012 <110473008+ARADDCC012@users.noreply.github.com> Date: Wed, 20 Nov 2024 14:29:32 +0000 Subject: [PATCH 6/7] Updated release file names when uploading to s3 --- backend/src/services/mirroredModel.ts | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/backend/src/services/mirroredModel.ts b/backend/src/services/mirroredModel.ts index 1eb16ea8c..6fc6232dc 100644 --- a/backend/src/services/mirroredModel.ts +++ b/backend/src/services/mirroredModel.ts @@ -330,16 +330,11 @@ async function addReleaseToZip(user: UserInterface, model: ModelDoc, release: Re zip.append(JSON.stringify(release.toJSON()), { name: `${baseUri}/releaseDocument.json` }) for (const file of files) { zip.append(JSON.stringify(file.toJSON()), { name: `${baseUri}/files/${file._id}/fileDocument.json` }) - await uploadToS3( - `${baseUri}/files/${file._id}/fileContent`, - (await downloadFile(user, file._id)).Body as stream.Readable, - user.dn, - { - modelId: model.id, - releaseId: release.id, - fileId: file.id, - }, - ) + await uploadToS3(file.path, (await downloadFile(user, file._id)).Body as stream.Readable, user.dn, { + modelId: model.id, + releaseId: release.id, + fileId: file.id, + }) } } catch (error: any) { throw InternalError('Error when generating the zip file.', { error }) From f3426f477ebbc01b9e1727ee1280f6ad0333a4fe Mon Sep 17 00:00:00 2001 From: ARADDCC012 <110473008+ARADDCC012@users.noreply.github.com> Date: Wed, 20 Nov 2024 14:36:47 +0000 Subject: [PATCH 7/7] Updated zip file release and file names --- backend/src/services/mirroredModel.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/backend/src/services/mirroredModel.ts b/backend/src/services/mirroredModel.ts index 6fc6232dc..debd4128a 100644 --- a/backend/src/services/mirroredModel.ts +++ b/backend/src/services/mirroredModel.ts @@ -325,11 +325,10 @@ async function addReleaseToZip(user: UserInterface, model: ModelDoc, release: Re log.debug('Adding release to zip file of releases.', { user, modelId: model.id, semver: release.semver }) const files: FileInterfaceDoc[] = await getFilesByIds(user, release.modelId, release.fileIds) - const baseUri = `releases/${release.semver}` try { - zip.append(JSON.stringify(release.toJSON()), { name: `${baseUri}/releaseDocument.json` }) + zip.append(JSON.stringify(release.toJSON()), { name: `releases/${release.semver}.json` }) for (const file of files) { - zip.append(JSON.stringify(file.toJSON()), { name: `${baseUri}/files/${file._id}/fileDocument.json` }) + zip.append(JSON.stringify(file.toJSON()), { name: `files/${file._id}.json` }) await uploadToS3(file.path, (await downloadFile(user, file._id)).Body as stream.Readable, user.dn, { modelId: model.id, releaseId: release.id,