Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

129 add file already in bundle directory #215

Merged
merged 12 commits into from
Jan 21, 2025

Conversation

beatrizsavinhas
Copy link
Contributor

@beatrizsavinhas beatrizsavinhas commented Jan 15, 2025

Description

Closes #129.

Changed

  • Change logic in housekeeper add file to check if the file exists in the bundle directory and skip hard linking if the given file is the one already in the bundle directory.
  • If there already is a file with the same name in the bundle directory, the command aborts and to file should be hard linked to the directory or added to housekeeper in order to not override the original file.
  • Removed function link_to_housekeeper_path as it is unused given the new logic.

Testing

How to test

  • Pytest tests included

Review

  • code approved by
  • tests executed by BSV
  • "Merge and deploy" approved by
    Thanks for filling in who performed the code review and the test!

This version is a:

  • MAJOR - when you make incompatible API changes
  • MINOR - when you add functionality in a backwards compatible manner
  • PATCH - when you make backwards compatible bug fixes or documentation/instructions

@beatrizsavinhas beatrizsavinhas marked this pull request as ready for review January 16, 2025 08:12
@beatrizsavinhas beatrizsavinhas requested a review from a team as a code owner January 16, 2025 08:12
@beatrizsavinhas beatrizsavinhas marked this pull request as draft January 16, 2025 08:13
@beatrizsavinhas beatrizsavinhas marked this pull request as ready for review January 17, 2025 13:52
Copy link
Contributor

@ChrOertlin ChrOertlin left a comment

Choose a reason for hiding this comment

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

Look good given the state of housekeeper. :)

Just wondering. I read through the issue again as well. What happens when we re-demux fastq files that had index errors and now need to be re-added in hk? Or anything similar where new files need to replace old ones.

housekeeper/include.py Outdated Show resolved Hide resolved
Copy link

@beatrizsavinhas
Copy link
Contributor Author

Look good given the state of housekeeper. :)

Just wondering. I read through the issue again as well. What happens when we re-demux fastq files that had index errors and now need to be re-added in hk? Or anything similar where new files need to replace old ones.

Thank you! :D

Isn't that situation unrelated with this case here? 🤔
This logic will allow the user to add an entry for a file in the housekeeper database when the file is already in the housekeeper bundle directory (basically skips the hard linking because the file is already in the hk directory).
In case there are already files in housekeeper that need to be replaced, the user would have to first remove the outdated files and then add the new ones.

@ChrOertlin
Copy link
Contributor

Look good given the state of housekeeper. :)
Just wondering. I read through the issue again as well. What happens when we re-demux fastq files that had index errors and now need to be re-added in hk? Or anything similar where new files need to replace old ones.

Thank you! :D

Isn't that situation unrelated with this case here? 🤔 This logic will allow the user to add an entry for a file in the housekeeper database when the file is already in the housekeeper bundle directory (basically skips the hard linking because the file is already in the hk directory). In case there are already files in housekeeper that need to be replaced, the user would have to first remove the outdated files and then add the new ones.

Indeed, if the user is the automation how would that work? Would the automation need to remove those files first as well? I am not 100% familiar with the issue so it might not be a problem at all. Just asking whether this has been considered or not.

@beatrizsavinhas
Copy link
Contributor Author

Checked and the command changed does not seem to be called by any automation or the housekeeper api in cg!

@beatrizsavinhas beatrizsavinhas merged commit 84be22e into master Jan 21, 2025
8 checks passed
@beatrizsavinhas beatrizsavinhas deleted the 129-add-file-already-in-bundle-directory branch January 21, 2025 13:15
@beatrizsavinhas
Copy link
Contributor Author

beatrizsavinhas commented Jan 21, 2025

Deployment

[beatriz.sa@hasta:~] [base] $ housekeeper-deploy
Are you sure you want to pull the latest Housekeeper image? [Y/n] y
Pulling the latest Housekeeper image which will be used in stage and prod
WARNING: Authentication token file not found : Only pulls of public images will succeed
INFO:    Starting build...
Getting image source signatures
Skipping fetch of repeat blob sha256:89320e7119e225692d79aaeb4989a18d7f97daafdb2b3782f84a0a8de31a09de
Skipping fetch of repeat blob sha256:3209946de9579ac14f7eaf9d21e81a6d09435da76479d8ebb28a4c445f34f8a3
Skipping fetch of repeat blob sha256:91ae80a0eefbaa4c934bee961631566b75946a0ad59cb9e8fa8bfdd9b6b3773b
Skipping fetch of repeat blob sha256:80bb75eec761350ae41910a383dff9097812ebd9917560d6d50c14d00210fc72
Copying blob sha256:e943e395a7594f1211b25a65062b0e3cffcb2c6309c72edfde3bf9f84f73914a
 93 B / 93 B [==============================================================] 0s
Copying blob sha256:3ebdc146ba0de08b121bcea78def4112c9cee04279b4f839a95b3859eb264885
 455.32 KiB / 455.32 KiB [==================================================] 0s
Copying blob sha256:2f6b22f2783b6f1177d7d5c8fcf81dd648a376d3e14515cfd5d21c717440e767
 69.23 MiB / 69.23 MiB [====================================================] 1s
Copying config sha256:42a5eb662cb7cf2667bac84b5f8204564c2779c3f27a28a3cd998e8379d879a9
 5.92 KiB / 5.92 KiB [======================================================] 0s
Writing manifest to image destination
Storing signatures
INFO:    Creating SIF file...
INFO:    Build complete: /home/proj/stage/singularity_containers/housekeeper_temp.sif
Successfully pulled the latest Housekeeper image.
mv: try to overwrite ‘/home/proj/stage/singularity_containers/housekeeper_latest.sif’, overriding mode 0750 (rwxr-x---)? y

Tested in stage and it seems to be working as intended!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding files to an included bundle that are already in the target.
2 participants