-
Notifications
You must be signed in to change notification settings - Fork 354
[ENG-9596] Server slowdown when adding many contributors #11420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/pbs-25-25
Are you sure you want to change the base?
[ENG-9596] Server slowdown when adding many contributors #11420
Conversation
8622ab5 to
996bba3
Compare
ihorsokhanexoft
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few notes:
| uid = item.get('_id') | ||
| user = user_map.get(uid) | ||
| email = item.get('user', {}).get('email', None) | ||
| full_name = item.get('full_name') or (user.fullname if user and not user.is_registered else None) | ||
| if not uid and not full_name: | ||
| raise exceptions.ValidationError(detail='A user ID or full name must be provided to add a contributor.') | ||
| child_payload.append({ | ||
| 'user_id': uid, | ||
| 'user': user, | ||
| 'email': email, | ||
| 'full_name': full_name, | ||
| 'send_email': send_email_default, | ||
| 'permissions': _perm(item), | ||
| 'bibliographic': item.get('bibliographic'), | ||
| 'index': item.get('_order') if '_order' in item else None, | ||
| }) | ||
| try: | ||
| child.add_contributors_registered_or_not(child_payload, auth=auth, save=True) | ||
| except ValidationError as e: | ||
| raise exceptions.ValidationError(detail=e.messages[0]) | ||
| except ValueError as e: | ||
| raise exceptions.NotFound(detail=e.args[0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this part repeats above, so let's create a separate method for parsing data
| # Use DRF list_serializer_class if it exists, otherwise use default JSONAPIListSerializer | ||
| meta = getattr(cls, 'Meta', None) | ||
| list_cls = getattr(meta, 'list_serializer_class', None) | ||
| if list_cls: | ||
| return list_cls(*args, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And would be nice to have a few tests to check data parsing inside of NodeContributorsBulkCreateListSerializer as it has a lot of logic
Purpose
optimize batch create contributors
Changes
see ticket comment section
QA Notes
Documentation
Side Effects
Ticket
https://openscience.atlassian.net/browse/ENG-9596