Skip to content

Conversation

@labkey-martyp
Copy link
Contributor

@labkey-martyp labkey-martyp commented Apr 7, 2023

Rationale

Move instead of copying files to Panorama Public.

https://www.labkey.org/Panorama%20Partners/Feature%20Requests/issues-details.view?issueId=47144

Related Pull Requests

Changes

  • Add PanoramaPublicFileImporter to handle file moves instead of copies.
  • Add PanoramaPublicMetadataImporter to handle run metadata on copy
  • Add PanoramaPublicSymlinkManager to handle symlinks on copies and other events
  • Add symlink handling for file and container listeners
  • Update and add new tests

labkey-martyp and others added 20 commits April 14, 2023 04:36
…tion if any of the datafileurls could not be fixed.

- PanoramaPublicSymlinkManager.moveAndSymLinkDirectory takes a Logger parameter so the log output can go to the job log
- Added a PanoramaPublicMetadataImporter. I moved some of the code out of CopyExperimentFinalTask into this class. This creates a row in the panoramapublic.experimentannotations table.  It runs before PanoramaPublicFileImporter so that if there is an error, e.g. datafileurls cannot be fixed, the container can be deleted to move files back to the source container.
- Updated test - import a document into a subfolder of the container file root.
… renamed / deleted is in the Panorama Public project. We don't expect folders in other projects to contain symlink targets.

- When handling folder rename (ContainerListener.propertyChange), pass the full paths of the old and renamed containers instead of just the folder names. Otherwise, it can lead to updating all symlinks that have the old folder name in the path.
- When deleting a folder, use ExperimentAnnotationsManager.getExperimentIncludesContainer(c) to lookup the experiment. This method will return the experiment that contains runs from the folder even if it is a subfolder of the folder where the experiment was created.
- When an experiment folder in Panorama Public is deleted, move the files back to next highest experiment version if one exists. Otherwise, move the files back to the source folder.
…s should only include the source container in the submitter's project as well as any containers with older versions of the data on Panorama Public.
…fileUrls. This should not be required anymore due to LabKey/targetedms#724.

Set filePathRoot on the copied expRun to be the target container's file root.
Log error if the data file path is unexpected, i.e. it does not contain "Run<runid>"
@labkey-martyp labkey-martyp marked this pull request as ready for review May 25, 2023 12:57
Copy link
Contributor

@labkey-jeckels labkey-jeckels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is too big to do a full validation just via code review. I know you've been working closely with Vagisha so I think there's good coverage on the higher level functionality, but let me know if there's anything in particular that warrants extra review on my end.

@labkey-jeckels labkey-jeckels merged commit 5bcf018 into release23.3-SNAPSHOT Jul 1, 2023
@labkey-jeckels labkey-jeckels deleted the 23.3_fb_copy_vs_mv_pano_files branch July 1, 2023 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants