Refactor SQS functionality and add new high-level methods #33
Conversation
This file contains hidden or 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
ghukill
approved these changes
Jan 2, 2025
ghukill
left a comment
ghukill
left a comment
There was a problem hiding this comment.
Seems like a good time to tackle some of these renamings and type tweaks (e.g. when required vs optional). Looks good!
Why these changes are being introduced: * The SQS functionality copied from wiley-deposits needs to be refactored to accommodate additional workflows. How this addresses that need: * Update ItemSubmission class to store item_identifier rather than metadata_s3_key as an attribute * Update ItemSubmission.upload_dspace_metadata method signature to accept s3_prefix * Add ItemSubmission.send_submission_message method and corresponding unit test * Rename SQSClient.create_dss_message_attributes param package_id > identifier for clarity * Update SQSClient.create_dss_message_body to handle multiple bitstreams * Rename method to SQSClient.validate_result_message to clarify the type of SQS message being validated * Update BaseWorkflow.s3_prefix type hinting * Update BaseWorkflow.run method and corresponding unit test * Refactor BaseWorkflow.item_submission_iter to account for the new ItemSubmission.item_identifier attribute * Add s3 to metadata_uri and bitstream_uris attribute names across repo for clarity Side effects of this change: * None Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/IN-1128
* Add links to DSS message spec in docstrings * Update tests after SimpleCSV PR merge
ehanson8
force-pushed
the
IN-1101-sqs-refactor
branch
from
6c93bdd to
69a88f3
Compare
Pull Request Test Coverage Report for Build 12602471343Details
💛 - Coveralls |
jonavellecuerdo
requested changes
Jan 3, 2025
Contributor
jonavellecuerdo
left a comment
jonavellecuerdo
left a comment
There was a problem hiding this comment.
Looking good! I have a few potential change requests.
* Rename identifier > item_identifier * Rename s3_prefix > prefix
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.
Purpose and background context
After trying to create higher-level methods, it became clear that the SQS functionality copied from
wiley-depositsneeded to be refactored.How can a reviewer manually see the effects of these changes?
Not possible yet
Includes new or updated dependencies?
NO
Changes expectations for external applications?
NO
What are the relevant tickets?
Developer
Code Reviewer(s)