FOLIOSYNC-7 Prepare files for Hyacinth Sync #38
Conversation
…or downloading single MARC based on hrid
…e 001 field as a filename
…gem to make a query, initialize yml file + dirs
elohanlon
left a comment
elohanlon
left a comment
There was a problem hiding this comment.
Just a few minor suggested changes (that mostly revolve around one suggested change in the extract_id method), but otherwise looks good!
| end | ||
|
|
||
| # Downloads all SRS MARC bibliographic records that have a 965 field that has a subfield $a value of '965hyacinth' | ||
| # AND were modified after the given `modified_since` time. |
There was a problem hiding this comment.
Minor comment update needed here, just because the comment references modified_since and the method argument is last_x_hours.
| def extract_id(marc_record) | ||
| field_001 = marc_record['fields']&.find { |f| f['001'] } | ||
| field_001 ? field_001['001'] : 'unknown' | ||
| end |
There was a problem hiding this comment.
It would be a problem if the record is missing an 001 (and hopefully that will never happen!), so I think it would be good to have this method return nil if no 001 is present instead of returning string 'unknown'. I left a couple of other comments above about the side effects of returning nil.
The overall restructuring I'm proposing will prevent the (hopefully rare/impossible) case where we would write our an "unknown.mrc" file to disk. And I think the code change would also make it clearer that a record with a missing 001 value is an exceptional case that would cause issues generally.
|
|
||
| def save_marc_record_to_file(marc_record) | ||
| config = Rails.configuration.folio_to_hyacinth | ||
| filename = extract_id(marc_record) |
There was a problem hiding this comment.
Later in this file, I proposed that extract_id return nil if an 001 isn't present. If this method returns nil here, I think it would be good to raise an exception (something like FolioSync::Exceptions::Missing001, which would extend FolioSyncException, and FolioSyncException extends StandardError so it would be caught by your line 27 rescue block).
| begin | ||
| save_marc_record_to_file(parsed_record) | ||
| rescue StandardError => e | ||
| record_id = extract_id(parsed_record) |
There was a problem hiding this comment.
| record_id = extract_id(parsed_record) | |
| record_id = extract_id(parsed_record) || 'unknown' |
Later in this file, I'm proposing that extract_id return nil when a 001 isn't found, so I'm suggesting this change to go along with that.
| source_record = @folio_client.find_source_record(instance_record_hrid: folio_hrid) | ||
| marc_record = source_record['parsedRecord']['content'] if source_record | ||
|
|
||
| unless has_965hyacinth_field?(marc_record) |
There was a problem hiding this comment.
Good move adding this check here!
|
@elohanlon Thanks for the review! I updated the logic to throw a custom exception if the 001 field is not present. I wasn't sure if that would ever be the case and I agree that it's safer to skip saving MARC if that happens. |
…y to handle timeouts; add rake task for testing creating and updating hyacinth records
…e_marc_records method
… github.com:cul/folio_sync into feature/FOLIOSYNC-8-port-hyacinth-client-to-foliosync
… HyacinthApi client under FolioSync class
… ignore line length warnings for gem files
…ent-to-foliosync
wtomaszewska
deleted the
feature/FOLIOSYNC-7-prepare-files-for-hyacinth-sync
branch
3 participants
Ticket FOLIOSYNC-7
Overview
This PR contains the first part of the work required for HYSYNC: retrieving recently modified MARC SRS records (with the option to fetch all records, regardless of the modification time) that have their 965$a field set to
965hyacinth. We'll download these MARC records locally before using them to assemble the data required by Hyacinth.Note: The main filtering logic lives in the Folio API gem and is currently part of this PR.
Other things added: