Use the change-manager API to determine file processing has completed #69
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mjgiarlo
commented
Jun 15, 2023
mjgiarlo
commented
Jun 15, 2023
| # If called too quickly, might get Failure(:not_found) | ||
| data_importer.status | ||
| => Failure(:pending) | ||
| data_importer.wait_until_complete | ||
| => Success() | ||
| data_importer.instance_hrid | ||
| => Success("in00000000010") | ||
| data_importer.instance_hrids |
There was a problem hiding this comment.
Multiple may be returned now!
mjgiarlo
commented
Jun 15, 2023
| # Failure(:not_found) if job is not found | ||
| def status | ||
| response_hash = client.get("/metadata-provider/jobSummary/#{job_execution_id}") | ||
| response_hash = client.get("/change-manager/jobExecutions/#{job_execution_id}") |
There was a problem hiding this comment.
Here's the API endpoint change.
mjgiarlo
commented
Jun 15, 2023
|
|
||
| return Failure(:error) if response_hash["totalErrors"].positive? | ||
| return Failure(:pending) if response_hash.dig("sourceRecordSummary", "totalCreatedEntities").zero? && response_hash.dig("sourceRecordSummary", "totalUpdatedEntities").zero? | ||
| return Failure(:pending) if !["COMMITTED", "ERROR"].include?(response_hash["status"]) |
There was a problem hiding this comment.
An "ERROR" approach means one or more records failed, but it does not mean they all fail. We probably need a more nuanced way to handle this eventually. Given there are zero consumers of this part of the Ruby gem, though, I was thinking it'd be fine to say "this is good enough for now."
There was a problem hiding this comment.
Put in a comment to this effect?
mjgiarlo
commented
Jun 15, 2023
| @@ -61,7 +59,7 @@ def default_wait_secs | |||
| end | |||
|
|
|||
| def default_timeout_secs | |||
| 5 * 60 | |||
| 10 * 60 | |||
There was a problem hiding this comment.
I needed to bump this up to run a test script with a real MARC file containing 1.1K records.
b1e8f6d to
8bbd15e
Compare
justinlittman
approved these changes
Jun 16, 2023
|
|
||
| return Failure(:error) if response_hash["totalErrors"].positive? | ||
| return Failure(:pending) if response_hash.dig("sourceRecordSummary", "totalCreatedEntities").zero? && response_hash.dig("sourceRecordSummary", "totalUpdatedEntities").zero? | ||
| return Failure(:pending) if !["COMMITTED", "ERROR"].include?(response_hash["status"]) |
There was a problem hiding this comment.
Put in a comment to this effect?
This commit updates our best understanding of how to navigate the Folio APIs to determine that a `processFiles` request has finished running. Adapted from sul-dlss/libsys-airflow#670 As part of this work, expand data import functionality to be able to handle multiple records.
8bbd15e to
334d319
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why was this change made? 🤔
This commit updates our best understanding of how to navigate the Folio APIs to determine that a
processFilesrequest has finished running. Adapted from sul-dlss/libsys-airflow#670As part of this work, expand data import functionality to be able to handle multiple records.
How was this change tested? 🤨
CI + run against stage with a test script