Skip to content

ETT-1288 Run fixity check on new storage#175

Draft
moseshll wants to merge 11 commits intomainfrom
ETT-1288_fixity
Draft

ETT-1288 Run fixity check on new storage#175
moseshll wants to merge 11 commits intomainfrom
ETT-1288_fixity

Conversation

@moseshll
Copy link
Contributor

@moseshll moseshll commented Mar 3, 2026

  • Add storage_name column to feed_audit and feed_audit_detail
  • main_repo_audit.pl now requires a --storage_name parameter
    • Expected values s3-truenas-ictc and s3-truenas-macc
  • HTFeed/Storage/LocalPairtree.pm and HTFeed/StorageAudit.pm also write feed_audit.storage_namd and feed_audit_detail.storage_name, respectivelt, but I'm not sure what values that can or should provide
  • Add a couple of happy path tests for the script as a whole.

moseshll added 8 commits March 3, 2026 11:38
- Add `storage_name` column to `feed_audit` and `feed_audit_detail`
- `main_repo_audit.pl` now requires a `--storage_name` parameter
  - Expected values `s3-truenas-ictc` and `s3-truenas-macc`
- `HTFeed/Storage/LocalPairtree.pm` and `HTFeed/StorageAudit.pm` also write `feed_audit.storage_namd` and `feed_audit_detail.storage_name`, respectivelt, but I'm not sure what values that can or should provide
- Add a couple of happy path tests for the script as a whole.
- New code is truenas_audit.pl
- TODO: several FIXMEs in the code, possible leftovers from main_repo_audit.pl inside truenas_audit.pl
@aelkiss aelkiss self-requested a review March 11, 2026 16:57
Copy link
Member

@aelkiss aelkiss left a comment

Choose a reason for hiding this comment

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

I've looked at this some; I want to look at the tests more, not sure I'll get to that today though.

my $last_touched = $zip_seconds;
$last_touched = $mets_seconds if defined $mets_seconds and (not defined $zip_seconds or $mets_seconds > $zip_seconds);

# FIXME: I don't know if this is needed and if it is this is old code from main_repo_audit.pl so it needs fixin'
Copy link
Member

Choose a reason for hiding this comment

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

I think this is worth keeping for now to verify symlinks from sdr1 to the "real" storage location are intact, although we can remove the $last_touched check.

# TODO populate image_size, page_count

# FIXME: is this right?? should we force it to one of {s3-truenas-ictc, s3-truenas-macc}?
my $storage_name = $self->{storage_name} || 'LocalPairtree';
Copy link
Member

Choose a reason for hiding this comment

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

Probably? Or rather, we can bail out if storage_name isn't set?

Copy link
Member

@aelkiss aelkiss left a comment

Choose a reason for hiding this comment

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

Tests look good; see comments about the backticks. Not critical, but maybe worth considering.


foreach my $storage_name (('s3-truenas-macc', 's3-truenas-ictc')) {
it "writes to feed_storage" => sub {
`bin/audit/truenas_audit.pl --md5 --storage_name $storage_name /tmp/sdr1`;
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to wrap the contents of truenas_audit.pl and do a main unless caller kind of thing at the end, to make it easier to test?

# filesystem but the `tmpdirs` random names will throw it off completely. Hence we
# copy to a location where we can put "sdr1" in the path.
File::Path::make_path('/tmp/sdr1/obj');
`cp -r $tmpdirs->{obj_dir}/* /tmp/sdr1/obj/`;
Copy link
Member

Choose a reason for hiding this comment

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

Might be worth using system rather than backticks since we're throwing away the return value; that also has the benefit of allowing stdout to go to console which could help in debugging issues with tests.

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.

2 participants