Skip to content

checksum-directory, publish-xenium-results: Introduce checksums folder mode - #568

Draft
ce10-sanger wants to merge 6 commits into
wtsi-npg:develfrom
ce10-sanger:finish-pre-2026-upload
Draft

checksum-directory, publish-xenium-results: Introduce checksums folder mode#568
ce10-sanger wants to merge 6 commits into
wtsi-npg:develfrom
ce10-sanger:finish-pre-2026-upload

Conversation

@ce10-sanger

Copy link
Copy Markdown
Contributor

Introduces a new checksumming mode - checksums folder - where instead of specifying a checksums file you specify a directory and the tools calculate location of the checksums file within the checksums folder.

Motivated by adopting checksumming in Xenium publishing and not wanting to change the interface of publish-xenium-results from accepting directories to publish one per line.

@ce10-sanger
ce10-sanger force-pushed the finish-pre-2026-upload branch from 2e51f1d to 40ee60e Compare June 23, 2026 09:01
@ce10-sanger
ce10-sanger marked this pull request as ready for review June 23, 2026 09:23
Supports two modes of specifying checksum locations:

- Explicitly specifying a file to write checksums to (`--md5sums-path`)
- Specifying a directory (`--checksums-directory`). Checksums are written to a

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

md5sums-directory?

- Explicitly specifying a file to write checksums to (`--md5sums-path`)
- Specifying a directory (`--checksums-directory`). Checksums are written to a
file within the directory that can be uniquely retrieved by other
npg-irods-python utilities.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

explain structure of dir of files with one file per dir


The output follows GNU coreutils md5sum format. Checksum files (*.md5) are
ignored. Files with existing checksums are skipped.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

simpler just to have one file?

Comment thread src/npg_irods/checksum.py
continue

with open(path, "rb") as f:
if file_path.is_symlink():

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fix problem at source rather than filtering out?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

decided to add as flag

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

label up as a workaround

@ce10-sanger
ce10-sanger marked this pull request as draft June 23, 2026 11:32
@ce10-sanger

Copy link
Copy Markdown
Contributor Author

moved back to draft - will return to decisions when learnt more from building xenium automation

@ce10-sanger

ce10-sanger commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Docs unclear whether directory of files per file or files per directory
--checksums-directory > --md5sums-directory
Simpler to just have one file per whole run?
Options

  • One file per Xenium
  • One file per invocation of publish-xenium-results
  • One file per time period?
    Considerations
  • Deletion/cache lifecycle
    Integrate checksumming into publish_directory
    Don't have a separate checksumming step, save/cache checksums as calculated by publish_directory
    Symlinks - fix problem at source rather than filtering out? Risk of data loss
    Value in testing handling of symlink directories
    Agreed would work on Xenium automation and return to these decisions

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.

1 participant