Skip to content

[ENG-9044] Add manage command to resync preprint dois v1 - #11617

Open
Vlad0n20 wants to merge 9 commits into
CenterForOpenScience:feature/pbs-26-15from
Vlad0n20:fix/ENG-9044
Open

[ENG-9044] Add manage command to resync preprint dois v1#11617
Vlad0n20 wants to merge 9 commits into
CenterForOpenScience:feature/pbs-26-15from
Vlad0n20:fix/ENG-9044

Conversation

@Vlad0n20

@Vlad0n20 Vlad0n20 commented Mar 2, 2026

Copy link
Copy Markdown
Contributor

Ticket

Purpose

Changes

Side Effects

QE Notes

CE Notes

Documentation

@cslzchen cslzchen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. In addition to my questions/comments:

  • Can we add logs of the output of the logs for you local run?
  • We should also work with CE to test this command with a copy of production DB.

Comment thread osf/management/commands/resync_preprint_dois_v1.py Outdated
Comment thread osf/management/commands/resync_preprint_dois_v1.py Outdated
Comment thread osf/management/commands/resync_preprint_dois_v1.py Outdated
Comment thread osf/management/commands/resync_preprint_dois_v1.py
Comment thread osf/management/commands/resync_preprint_dois_v1.py Outdated
@Vlad0n20 Vlad0n20 changed the title Add manage command to resync preprint dois v1 [ENG-9044] Add manage command to resync preprint dois v1 Mar 9, 2026

@cslzchen cslzchen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

  • Make sure we test this against a copy of prod DB before running this on prod.
    • If there are errors, need look further on why they fail, and probably another fix.
    • Test if the default batch size is good, how long does it take, and also see how many times we need to repeat this.
  • Make sure whoever uses this command knows that we need to repeatedly run this command.

Comment on lines +113 to +117
help=(
'Maximum number of preprints to process per run (default: 500). '
'The command processes the first N eligible preprints and exits; '
're-run the command to continue with the next batch.'
),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. This is an OK alternative to the loop I suggested. Just need to make sure whoever runs this command is aware of the fact that they need to keep running this command until none exists. Cc @adlius for your input on this.

@adlius
adlius changed the base branch from feature/pbs-26-2 to feature/pbs-26-6 April 7, 2026 13:13
@Vlad0n20
Vlad0n20 force-pushed the fix/ENG-9044 branch 2 times, most recently from e803a0c to e1d4931 Compare April 14, 2026 11:46
@antkryt

antkryt commented May 4, 2026

Copy link
Copy Markdown
Contributor

LGTM

@mkovalua mkovalua left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, @Vlad0n20, please change target branch to current one feature/pbs-26-9, I have no permission to do it

@Vlad0n20
Vlad0n20 changed the base branch from feature/pbs-26-6 to feature/pbs-26-9 May 13, 2026 09:02
@Vlad0n20
Vlad0n20 changed the base branch from feature/pbs-26-9 to feature/pbs-26-15 July 28, 2026 14:13
Comment thread api/users/views.py Outdated

user.date_last_logged_in = timezone.now()
user.external_identity[provider][provider_id] = 'VERIFIED'
if provider.lower() in OSFUser.SOCIAL_FIELDS:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this part of this ticket?

@adlius adlius left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I have a couple of suggestions. Since Crossref has a limit of 10000 submissions from a single user, might as well set the default batch size to 1000 and no rate limit. Instead let's run the task every five minutes so that we can send about 12000 requests per hour.

Also there are some commits from other branches that is in this PR.

Comment thread osf_tests/test_user.py Outdated
assert len(user.system_tags) == 1

tag = Tag.all_tags.get(name=tag_name, system=True)
tag = Tag.all_tags.get(name=tag_name.lower(), system=True)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. Is this part of this ticket?

return qs


def resync_preprint_dois_v1(dry_run=True, batch_size=500, rate_limit=100, provider_id=None):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rate limit of 100 is perhaps to restrictive. According to conversations with Crossref, they have a limit of 10000 pending items for each user.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A 500 batch size is also perhaps too small.


if rate_limit and not record_number % rate_limit:
logger.info(f'Rate limit reached at {record_number} preprints, sleeping {RATE_LIMIT_SLEEP}s')
time.sleep(RATE_LIMIT_SLEEP)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am afraid this would clog the celery worker when this is run as a celery beat task.

@brianjgeiger brianjgeiger left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive by:

  1. What queue are the tasks going onto? The default queue? We should probably ensure that they go on the Low Priority queue to keep from clogging the pipes too much.
  2. If we have a cron job that happens every five minutes, how are you going to run this in a dry-run scenario to make sure it's going to work before the cron job runs? We might want to rethink our strategy here.

@adlius adlius left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some issues found during local testing.

Comment thread website/identifiers/clients/crossref.py

@adlius adlius left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more issues to fix.

def create_identifier(self, preprint, category, include_relation=True):
if category == 'doi':
metadata = self.build_metadata(preprint, include_relation)
metadata = self.build_metadata(preprint, include_relation, include_unversioned_doi=True)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more issue found during local testing: on an unmoderated preprint provider, when a user creates a new version but never publish that version, build_unversioned_posted_content would read preprint.get_guid().referent, which hands back that unpublished draft instead of the preprint actually being deposited. This is problematic because the unpublished draft will have date_published to be None and that would fail when building the payload.

Comment thread api/crossref/views.py
@@ -48,6 +49,23 @@ def post(self, request):
if record.get('status').lower() == 'success' and doi:
msg = record.find('msg').text
created = bool(msg == 'Successfully added')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So a DOI in the doi_unversioned category is only written when Crossref replies Successfully added. This could be a potential issue because if the DOI
is already registered, the email probably comes back with something like Successfully updated, so for those preprints
this branch never runs.

This would affect legacy preprints that exists before versioning was a thing. Their base guid is their original guid, so the DOI deposit here is the one registered years ago.

I checked against a local copy of prod db. One of preprint 22hq2 has 10.31219/osf.io/22hq2 already registered with crossref. So it's possible that the crossref response email wouldn't contain Successfully added but Successfully updated or some other language. Will need to confirm if that is indeed the case.

If that is the case, one possible fix is that for an unversioned DOI, store doi_unversioned on any successful record rather than only on Successfully added.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is another problem: if we run the resync task every five minutes, and if the preprint that was resynced in the last run still haven't received an email back confirming the update of the DOI, then in the next run it will be included in the resync task again. So we'd have to rethink how we should do this. One idea suggested by @cslzchen is that we follow the example in this PR and monitory the number of active tasks and spawn new tasks if the number of active task falls below a certain number (say 1000). In this case, we don't have to worry about running the task every five minutes.

).exclude(
tags__name='qatest',
tags__system=True,
).select_related('provider').distinct()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we probably need an order_by here otherwise the ordering of the queryset would be undeterministic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants