[ENG-9828] - Backfill CedarMetadataRecord from CollectionSubmission custom metadata#11740
[ENG-9828] - Backfill CedarMetadataRecord from CollectionSubmission custom metadata#11740Vlad0n20 wants to merge 5 commits into
Conversation
| logger.exception(e) | ||
| sentry.log_exception(e) | ||
|
|
||
| def sync_cedar_metadata(self): |
There was a problem hiding this comment.
since this overlaps #11735 , why not put them in the same PR, or make this one based on that one, or move the sync_cedar_metadata definition into a separate commit shared by both PRs... any way to avoid defining the same function twice and having to go back and forth to check they're identical until the inevitable merge conflict...
Vlad0n20
force-pushed
the
feature/ENG-9828
branch
from
7e7f928 to
dc1bbae
Compare
| logger.info(f'[DRY RUN] Would sync cedar metadata for submission {submission._id}') | ||
| continue | ||
| try: | ||
| submission.sync_cedar_metadata() |
There was a problem hiding this comment.
well now i'm confused -- sync_cedar_metadata doesn't seem to exist?
| nonlocal call_count | ||
| call_count += 1 | ||
| if call_count == 1: | ||
| raise Exception('simulated error') |
There was a problem hiding this comment.
nit: might be easier/cleaner to give side_effect a list to raise on the first call and succeed (return None) on the second:
mock.side_effect = [Exception('simulated error'), None]
https://docs.python.org/3.13/library/unittest.mock.html#unittest.mock.Mock.side_effect
| with mock.patch.object(CollectionSubmission, 'sync_cedar_metadata', sync_side_effect): | ||
| copy_collection_submission_metadata_to_cedar() | ||
|
|
||
| assert call_count == 2 |
There was a problem hiding this comment.
nit: the mock already has this, no need to do it by hand -- _mock_sync_side_effect.call_count
| copy_collection_submission_metadata_to_cedar() | ||
|
|
||
| record = CedarMetadataRecord.objects.get(guid=submission.guid, template=cedar_template) | ||
| assert record.metadata == {'status': 'new', '@context': cedar_template.cedar_id} |
There was a problem hiding this comment.
not sure this @context is correct -- when you add sync_cedar_metadata, make sure its output is valid
Vlad0n20
force-pushed
the
feature/ENG-9828
branch
from
8aca88a to
29e9b04
Compare
3 participants
Ticket
Purpose
Changes
Side Effects
QE Notes
CE Notes
Documentation