Conversation
- 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
left a comment
There was a problem hiding this comment.
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' |
There was a problem hiding this comment.
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'; |
There was a problem hiding this comment.
Probably? Or rather, we can bail out if storage_name isn't set?
aelkiss
left a comment
There was a problem hiding this comment.
Tests look good; see comments about the backticks. Not critical, but maybe worth considering.
t/truenas_audit.t
Outdated
|
|
||
| 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`; |
There was a problem hiding this comment.
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?
t/truenas_audit.t
Outdated
| # 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/`; |
There was a problem hiding this comment.
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.
storage_namecolumn tofeed_auditandfeed_audit_detailmain_repo_audit.plnow requires a--storage_nameparameters3-truenas-ictcands3-truenas-maccHTFeed/Storage/LocalPairtree.pmandHTFeed/StorageAudit.pmalso writefeed_audit.storage_namdandfeed_audit_detail.storage_name, respectivelt, but I'm not sure what values that can or should provide