-
Notifications
You must be signed in to change notification settings - Fork 4
(Large) Provenance MVP #630
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
…feature/network-graph
…feature/network-graph
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.
Moved unchanged from App.tsx
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.
Moved unchanged from App.tsx
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.
Moved unchanged from components/FileDetails
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.
This was used a LOT in a bunch of places so I replaced all I could find with this
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.
Moved unchanged from App
|
|
||
| await Promise.all( | ||
| filesToDownload.map(async (file) => { | ||
| const isStandardS3Url = file.path.includes("amazonaws.com"); |
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.
This logic around figuring out paths and when stuff was on the cloud was moved into fileDownloadService since this was duplicated in a few places with slight differences causing varied behavior that was messing with my testing.
| ): Promise<DownloadResult> { | ||
| let downloadUrl: string; | ||
|
|
||
| if (fileInfo.path.endsWith(".zarr")) { |
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.
This was also a victim of handling downloads differently and had its logic migrated.
| onProgress?: (transferredBytes: number) => void, | ||
| destination?: string | ||
| ): Promise<DownloadResult> { | ||
| if (this.isS3Url(fileInfo.path)) { |
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.
This was also a victim of handling downloads differently and had its logic migrated.
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.
Don't miss this file!! Holds most of the new complexity
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.
This came up in my testing quite a bit since some provenance actions were causing this (they shouldn't and I fixed each instance of it happening as it they came but they were and an unseemly default error page was appearing) so this is just a simple error page I added to replace the default from react-router that we can improve on later should we want to.
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.
This hook is very much changed. This previously was checking the remote server itself then returning a callback. Instead, all this does is return a callback and the check for the remote server now happens in interaction/logics to avoid this check happening more than once per application load. Previously it was happening several times since the component this hook lived in was tied to what created the context menu for file interactions which gets destroyed in certain scenarios. This got updated in this changeset because the sheer amount of logs this was generating was very distracting
|
How would this look if each zarr had 10 segmented cell images? Seems like it would be hard to fit/display that. That is more a UX question that a code one, though. |
Context
We want to provide users the ability to show context about how files relate to each other AKA "provenance" this includes things like how a file was generated (ex. script) or more conceptual things how a file relates to a publication.
(Shoutout @aswallace started this implementation :) )
Changes
Lots of changes! New components for generating the provenance graph were created, though much of the rendering is managed by
xyflow(seecomponents/NetworkGraph). Logical entities were also created to hold complexity (seeentity/Graph). Those are the major changes you should check out. However, additional cleanup to some busy components was done to alleviate some duplicate work that needed to happen in the graph components & to make testing easier.Testing
Lots of manual input of provenance & new unit tests. The lower level components like
FileNodeandMetadataNodecan't be tested using our current testing system unfortunately - hopefully this can be remedied in the upcoming improvements to our infrastructure.Try it here using a small, but high fidelity provenance dataset. Working to get a provenance table going for using with the AICS FMS source directly
Example using the above linked dataset:

This was created using this provenance source:

& this dataset:
