[ENG-7759] Add new task to update node metadata with verified_links #11078
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
Vlad0n20
force-pushed
the
feature/ENG-7759
branch
from
3268ba0 to
709bef4
Compare
cslzchen
requested changes
Apr 8, 2025
…y task to update project metadata to Datacite. Write tests
cslzchen
reviewed
Apr 17, 2025
There was a problem hiding this comment.
Partial review, don't respond yet:
- I picked the model update part and created a new [ENG-7759] Part 1: Add
addon_verified_resource_linkstonode#11099 - I don't think we need the links in the node/registration serializer. Let me know if you ran into anything I am not aware of which requires us to update serializer.
- Task should receive links/types and save the data to the node
- If possible, the save should also trigger automatic node identifier update
- Finally a nitpicking suggestion on condition and loop statements
| @@ -387,6 +388,8 @@ class AbstractNode(DirtyFieldsMixin, TypedModel, AddonModelMixin, IdentifierMixi | |||
|
|
|||
| schema_responses = GenericRelation('osf.SchemaResponse', related_query_name='nodes') | |||
|
|
|||
| verified_resource_links = DateTimeAwareJSONField(null=True, blank=True) | |||
There was a problem hiding this comment.
Renamed and updated options in my PR
addon_verified_resource_links = DateTimeAwareJSONField(default=dict, blank=True)| @@ -17,3 +17,29 @@ def task__update_doi_metadata_on_change(self, target_guid): | |||
| @celery_app.task(ignore_results=True) | |||
| def update_doi_metadata_on_change(target_guid): | |||
| task__update_doi_metadata_on_change(target_guid) | |||
|
|
|||
| @celery_app.task(bind=True, max_retries=5, acks_late=True) | |||
| def task__update_doi_metadata_with_verified_links(self, target_guid): | |||
There was a problem hiding this comment.
The task misses one important data: which is the links and types sent from GV. The task expects them to come in as arguments and the task should update the node. Otherwise, identifier update and Datacite tree-walk will see an empty dict.
In addition, I am wondering if we can make identifier update automatic when this field is saved/updated on node.
| verified_links = getattr(osf_item, 'verified_resource_links', None) | ||
| if verified_links: | ||
| for link, resource_type in verified_links.items(): | ||
| if link and isinstance(link, str) and smells_like_iri(link): |
There was a problem hiding this comment.
I guess this check is here to make sure link looks like a link, which is good.
- Is
smells_like_iri()precise enough? (i.e. not too loose or too strict) - If so, it already checks the
strtype so we can skip it.
| 'resourceTypeGeneral': resource_type | ||
| }) | ||
| else: | ||
| logger.warning('skipping non-URL verified link "%s"', link) |
There was a problem hiding this comment.
Turn this into a sentry message. In addition, let's make it Pythonic:
if not verified_links:
return
for ...:
if <false condition>:
# log sentry
continue
self.visit(...)
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
Add new task to update node metadata with verified_links
Changes
QA Notes
Please make verification statements inspired by your code and what your code touches.
What are the areas of risk?
Any concerns/considerations/questions that development raised?
Documentation
Side Effects
Ticket
https://openscience.atlassian.net/browse/ENG-7759?atlOrigin=eyJpIjoiMDQ3ZTEzYjU3ODU4NDVlZThiYmRiNWViMmQ3MzIzOWYiLCJwIjoiaiJ9