From 6215d9ec35b0c75ed3fc31a27adaa1a379a7c220 Mon Sep 17 00:00:00 2001 From: Philip Garrison Date: Wed, 18 Dec 2024 09:56:04 -0800 Subject: [PATCH 1/4] 367 FMS file_path is the cloud path (tests failing) --- .../FileDetails/FileAnnotationList.tsx | 16 -------------- packages/core/constants/index.ts | 4 ++-- packages/core/entity/FileDetail/index.ts | 22 +++++++++++++------ 3 files changed, 17 insertions(+), 25 deletions(-) diff --git a/packages/core/components/FileDetails/FileAnnotationList.tsx b/packages/core/components/FileDetails/FileAnnotationList.tsx index f5dea66f..0c3846d6 100644 --- a/packages/core/components/FileDetails/FileAnnotationList.tsx +++ b/packages/core/components/FileDetails/FileAnnotationList.tsx @@ -98,22 +98,6 @@ export default function FileAnnotationList(props: FileAnnotationListProps) { />, ]; - // Special case for file paths: we want to display both the "canonical" FMS path - // (i.e. POSIX path held in the database; what we have an annotation for) - // as well as the path at which the file is *actually* accessible on _this_ computer ("local" file path) - if (annotation.name === AnnotationName.FILE_PATH) { - if (fileDetails.path !== fileDetails.cloudPath) { - ret.push( - - ); - } - } - return ret; }, [] as JSX.Element[]); }, [annotations, fileDetails, isLoading, localPath]); diff --git a/packages/core/constants/index.ts b/packages/core/constants/index.ts index 75d289a4..ff53eb69 100644 --- a/packages/core/constants/index.ts +++ b/packages/core/constants/index.ts @@ -26,9 +26,9 @@ export const TOP_LEVEL_FILE_ANNOTATIONS = [ type: AnnotationType.STRING, }), new Annotation({ - annotationDisplayName: "File Path (Canonical)", + annotationDisplayName: "File Path (Cloud)", annotationName: AnnotationName.FILE_PATH, - description: "Path to file in storage.", + description: "Path to file in the cloud.", type: AnnotationType.STRING, }), new Annotation({ diff --git a/packages/core/entity/FileDetail/index.ts b/packages/core/entity/FileDetail/index.ts index ac944805..5e59cb5b 100644 --- a/packages/core/entity/FileDetail/index.ts +++ b/packages/core/entity/FileDetail/index.ts @@ -4,6 +4,8 @@ import { renderZarrThumbnailURL } from "./RenderZarrThumbnailURL"; const RENDERABLE_IMAGE_FORMATS = [".jpg", ".jpeg", ".png", ".gif"]; +const AICS_FMS_S3_BUCKET = "production.files.allencell.org"; + /** * Expected JSON response of a file detail returned from the query service. Example: * { @@ -79,7 +81,11 @@ export default class FileDetail { const pathWithoutDrive = path.replace("/allen/programs/allencell/data/proj0", ""); // Should probably record this somewhere we can dynamically adjust to, or perhaps just in the file // document itself, alas for now this will do. - return `https://s3.us-west-2.amazonaws.com/production.files.allencell.org${pathWithoutDrive}`; + return `https://s3.us-west-2.amazonaws.com/${AICS_FMS_S3_BUCKET}${pathWithoutDrive}`; + } + + private static convertAicsS3PathToHttpUrl(path: string): string { + return `https://s3.us-west-2.amazonaws.com/${path}`; } constructor(fileDetail: FmsFile, uniqueId?: string) { @@ -116,6 +122,13 @@ export default class FileDetail { if (path === undefined) { throw new Error("File Path is not defined"); } + + // AICS FMS files have paths like staging.files.allencell.org/130/b23/bfe/117/2a4/71b/746/002/064/db4/1a/danny_int_test_4.txt + if (typeof path === "string" && path.startsWith(AICS_FMS_S3_BUCKET)) { + return FileDetail.convertAicsS3PathToHttpUrl(path) as string; + } + + // Otherwise just return the path as is and hope for the best return path as string; } @@ -128,12 +141,7 @@ export default class FileDetail { } public get cloudPath(): string { - // Can retrieve a cloud like path for AICS FMS files - if (this.path.startsWith("/allen")) { - return FileDetail.convertAicsDrivePathToAicsS3Path(this.path); - } - - // Otherwise just return the path as is and hope for the best + // AICS FMS files' paths are cloud paths return this.path; } From 160c0fe382132c28bcc8cfb59241dea3f7b54353 Mon Sep 17 00:00:00 2001 From: Philip Garrison Date: Thu, 19 Dec 2024 17:41:43 -0800 Subject: [PATCH 2/4] 367 Read cloud path from FileDetail object with http://... prefix --- .../FileDetails/FileAnnotationList.tsx | 9 ++++--- .../test/FileAnnotationList.test.tsx | 24 +++++++++---------- packages/core/entity/FileDetail/index.ts | 2 +- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/packages/core/components/FileDetails/FileAnnotationList.tsx b/packages/core/components/FileDetails/FileAnnotationList.tsx index 0c3846d6..42600287 100644 --- a/packages/core/components/FileDetails/FileAnnotationList.tsx +++ b/packages/core/components/FileDetails/FileAnnotationList.tsx @@ -88,7 +88,12 @@ export default function FileAnnotationList(props: FileAnnotationListProps) { } } - const ret = [ + if (annotation.name === AnnotationName.FILE_PATH) { + // Display the full http://... URL + annotationValue = fileDetails.cloudPath; + } + + return [ ...accum, , ]; - - return ret; }, [] as JSX.Element[]); }, [annotations, fileDetails, isLoading, localPath]); diff --git a/packages/core/components/FileDetails/test/FileAnnotationList.test.tsx b/packages/core/components/FileDetails/test/FileAnnotationList.test.tsx index 22db100c..3a2be2b4 100644 --- a/packages/core/components/FileDetails/test/FileAnnotationList.test.tsx +++ b/packages/core/components/FileDetails/test/FileAnnotationList.test.tsx @@ -14,7 +14,7 @@ import { AnnotationType } from "../../../entity/AnnotationFormatter"; describe("", () => { describe("file path representation", () => { - it("has both canonical file path and file path adjusted to OS & allen mount point", async () => { + it("has both cloud file path and local file path adjusted to OS & allen mount point", async () => { // Arrange const hostMountPoint = "/some/path"; @@ -46,7 +46,7 @@ describe("", () => { }); const filePathInsideAllenDrive = "path/to/MyFile.txt"; - const filePath = `production.allencell.org/${filePathInsideAllenDrive}`; + const filePath = `production.files.allencell.org/${filePathInsideAllenDrive}`; const fileDetails = new FileDetail({ file_path: filePath, file_id: "abc123", @@ -58,8 +58,6 @@ describe("", () => { ], }); - const localPath = `${hostMountPoint}/${filePathInsideAllenDrive}`; - // Act const { findByText } = render( @@ -69,16 +67,16 @@ describe("", () => { // Assert for (const cellText of [ - "File Path (Canonical)", - filePath, + "File Path (Cloud)", + `https://s3.us-west-2.amazonaws.com/${filePath}`, "File Path (Vast)", - localPath, + `${hostMountPoint}/${filePathInsideAllenDrive}`, ]) { expect(await findByText(cellText)).to.not.be.undefined; } }); - it("has only canonical file path when no allen mount point is found", () => { + it("has only cloud file path when no allen mount point is found", () => { // Arrange class FakeExecutionEnvService extends ExecutionEnvServiceNoop { public formatPathForHost(posixPath: string): Promise { @@ -100,7 +98,7 @@ describe("", () => { }); const filePathInsideAllenDrive = "path/to/MyFile.txt"; - const filePath = `/allen/${filePathInsideAllenDrive}`; + const filePath = `production.files.allencell.org/${filePathInsideAllenDrive}`; const fileDetails = new FileDetail({ file_path: filePath, file_id: "abc123", @@ -119,9 +117,11 @@ describe("", () => { // Assert expect(() => getByText("File Path (Vast)")).to.throw(); - ["File Path (Canonical)", filePath].forEach((cellText) => { - expect(getByText(cellText)).to.not.be.undefined; - }); + ["File Path (Cloud)", `https://s3.us-west-2.amazonaws.com/${filePath}`].forEach( + (cellText) => { + expect(getByText(cellText)).to.not.be.undefined; + } + ); }); }); }); diff --git a/packages/core/entity/FileDetail/index.ts b/packages/core/entity/FileDetail/index.ts index 5e59cb5b..dd7da28d 100644 --- a/packages/core/entity/FileDetail/index.ts +++ b/packages/core/entity/FileDetail/index.ts @@ -81,7 +81,7 @@ export default class FileDetail { const pathWithoutDrive = path.replace("/allen/programs/allencell/data/proj0", ""); // Should probably record this somewhere we can dynamically adjust to, or perhaps just in the file // document itself, alas for now this will do. - return `https://s3.us-west-2.amazonaws.com/${AICS_FMS_S3_BUCKET}${pathWithoutDrive}`; + return FileDetail.convertAicsS3PathToHttpUrl(`${AICS_FMS_S3_BUCKET}${pathWithoutDrive}`); } private static convertAicsS3PathToHttpUrl(path: string): string { From 66f572c2f0f8bc49b926c514dc270a587bc3f334 Mon Sep 17 00:00:00 2001 From: Philip Garrison Date: Fri, 20 Dec 2024 11:01:52 -0800 Subject: [PATCH 3/4] 367 Hoist AICS_FMS_S3_URL_PREFIX to top-level constant --- packages/core/entity/FileDetail/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/entity/FileDetail/index.ts b/packages/core/entity/FileDetail/index.ts index dd7da28d..ab2a8f1e 100644 --- a/packages/core/entity/FileDetail/index.ts +++ b/packages/core/entity/FileDetail/index.ts @@ -5,6 +5,7 @@ import { renderZarrThumbnailURL } from "./RenderZarrThumbnailURL"; const RENDERABLE_IMAGE_FORMATS = [".jpg", ".jpeg", ".png", ".gif"]; const AICS_FMS_S3_BUCKET = "production.files.allencell.org"; +const AICS_FMS_S3_URL_PREFIX = "https://s3.us-west-2.amazonaws.com/"; /** * Expected JSON response of a file detail returned from the query service. Example: @@ -85,7 +86,7 @@ export default class FileDetail { } private static convertAicsS3PathToHttpUrl(path: string): string { - return `https://s3.us-west-2.amazonaws.com/${path}`; + return `${AICS_FMS_S3_URL_PREFIX}${path}`; } constructor(fileDetail: FmsFile, uniqueId?: string) { From 88173323832b26611f456ca2a1aaf2638264255f Mon Sep 17 00:00:00 2001 From: Philip Garrison Date: Fri, 20 Dec 2024 11:04:09 -0800 Subject: [PATCH 4/4] 367 More detailed file_path comment --- packages/core/entity/FileDetail/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/entity/FileDetail/index.ts b/packages/core/entity/FileDetail/index.ts index ab2a8f1e..a21a22fd 100644 --- a/packages/core/entity/FileDetail/index.ts +++ b/packages/core/entity/FileDetail/index.ts @@ -124,7 +124,8 @@ export default class FileDetail { throw new Error("File Path is not defined"); } - // AICS FMS files have paths like staging.files.allencell.org/130/b23/bfe/117/2a4/71b/746/002/064/db4/1a/danny_int_test_4.txt + // AICS FMS files have paths like this in fileDetail.file_path: + // staging.files.allencell.org/130/b23/bfe/117/2a4/71b/746/002/064/db4/1a/danny_int_test_4.txt if (typeof path === "string" && path.startsWith(AICS_FMS_S3_BUCKET)) { return FileDetail.convertAicsS3PathToHttpUrl(path) as string; }