sul-dlss / dor-services-app

A Rails application exposing Digital Object Registry functions as a RESTful HTTP API
https://sul-dlss.github.io/dor-services-app/
Other
3 stars 2 forks source link

add report for auditing contents of technical metadata database #4988

Closed jmartin-sul closed 4 months ago

jmartin-sul commented 4 months ago

Why was this change made? 🤔

to run the audit described in https://github.com/sul-dlss/technical-metadata-service/issues/515 (will close that issue once the report has actually been run)

How was this change tested? 🤨

on QA. once the related puppet changes were applied and the supporting shared_configs updated, i scped the report code over. then tested, fixed, refactored for usability, tested, fixed, repeated, until this felt like it'd be likely to run through on prod (or produce helpful logging if it didn't).

i did one full run on QA with this code (448 druids with techMD inconsistencies were found), and also ran both the test report and the single druid method.

example test report stdout (same stdout as full report would be, just limited to 20 druids audited):

$ bin/rails r -e p 'AuditTechnicalMetadataFileList.test_report'
druid:bk509qs2277: found technical-metadata-service database; inconsistencies with v11 cocina: {"missing_filenames"=>["asawa.jp2", "maps-africa.jp2", "moore.jp2", "hay.jp2"]}
druid:ww089jm4506: not found in technical-metadata-service database
druid:wy216td0310: found technical-metadata-service database; inconsistencies with v1 cocina: {"missing_filenames"=>["example.jp2"]}
druid:xb970cg0663: found technical-metadata-service database; inconsistencies with v1 cocina: {"missing_filenames"=>["example.jp2"]}
druid:gv185zf2539: not found in technical-metadata-service database

progress logging worked, as did the other info and debug logging.

example running on a single druid:

$ bin/rails r -e p 'AuditTechnicalMetadataFileList.audit_one_druid(druid: "druid:ww089jm4506")'
druid:ww089jm4506: not found in technical-metadata-service database

âš¡ âš  If this change has cross service impact, including data writes to shared file systems, run integration tests and/or test in [stage|qa] environment, in addition to specs. âš¡

jmartin-sul commented 4 months ago

interestingly, the QA audit turned up some unexpected files in techMD DB, and some instances where file names matched for a druid but md5 differed. i was only expecting missing files, but i wasn't sure enough of my understanding of the original problem for that to be a strong expectation. either way, glad i did had the techMD endpoint return all 3 categories of problem.

jmartin-sul commented 4 months ago

added a comment to settings.yml, got rid of the Faraday config cruft in #techmd_connection, added a comment to the confusing methods list in #retry_options. squashed that into the existing commit, but here's the diff from before i pushed for easy review:

% git diff origin/audit-techmd HEAD
diff --git a/app/reports/audit_technical_metadata_file_list.rb b/app/reports/audit_technical_metadata_file_list.rb
index aa94cc72..483620d5 100644
--- a/app/reports/audit_technical_metadata_file_list.rb
+++ b/app/reports/audit_technical_metadata_file_list.rb
@@ -154,6 +154,9 @@ class AuditTechnicalMetadataFileList
   def retry_options
     {
       max: 3,
+      # The default retriable/idempotent HTTP methods, plus POST. As of May 2024, POST is the only
+      # HTTP method this class actually uses (for a read-only operation, but the file list parameter
+      # might exceed the traditional max for total URL query param size, hence POST instead of GET).
       methods: %i[delete get head options put post]
     }
   end
@@ -162,7 +165,6 @@ class AuditTechnicalMetadataFileList
     @techmd_connection ||= Faraday.new(Settings.technical_metadata.url) do |builder|
       builder.request :retry, retry_options
       builder.use Faraday::Request::UrlEncoded
-      # builder.use Faraday::Response::RaiseError # raise exceptions on 40x, 50x responses
       builder.adapter Faraday.default_adapter
       builder.headers[:user_agent] = "dor-services-app #{Socket.gethostname}"
       builder.headers['Authorization'] = "Bearer #{Settings.technical_metadata.token}"
diff --git a/config/settings.yml b/config/settings.yml
index e2593ca8..a3fc94aa 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -74,6 +74,9 @@ purl_fetcher:
   url: 'https://purl-fetcher.example.edu'
   token: 'fake-token'

+# As of may 2024, this is just here to support auditing contents of
+# technical-metadata-service. If the report that uses it (AuditTechnicalMetadataFileList)
+# is removed, see whether this section can be removed.
 technical_metadata:
   url: 'https://techmd-svc.example.edu'
   token: 'secret-token'

merging since approved and i think i addressed the comments, but happy to revisit the retry_options thing or whatever else in a follow up PR if you'd like.