Skip to content

[ENG-8186] Storage allocation for draft registrations #11186

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

Open
wants to merge 24 commits into
base: feature/pbs-25-13
Choose a base branch
from

Conversation

ihorsokhanexoft
Copy link
Contributor

@ihorsokhanexoft ihorsokhanexoft commented Jun 17, 2025

SHOULD BE MERGED AFTER ENG-8048 WHICH CONTAINS ADDONS FIX THAT IMPACTS DISK USAGE CALCULATIONS IN ARCHIVER

Purpose

Admins should be able to configure storage limits for individual draft registrations

Changes

Updated draft registration admin page, added one field to DraftRegistration model, take into account draft registration storage limits when archive registration
Updated an email template to insert draft registration storage limit

Notes:

  1. MAX_ARCHIVE_SIZE can be removed from defaults.py on envs when it's deployed
  2. User tags are still used to provide users an unlimited storage

Ticket

https://openscience.atlassian.net/browse/ENG-8186?atlOrigin=eyJpIjoiYzM0ODM1ZWRhOWMzNGY0OGFiNzZkMWM2M2FkN2JmOTQiLCJwIjoiaiJ9

Copy link
Collaborator

@brianjgeiger brianjgeiger left a comment

Choose a reason for hiding this comment

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

A little more involved than I was expecting, but it looks like it should work. One question: does the draft registration inherit the custom storage allocation value from the node that it's being registered from? If not, it probably should.

@ihorsokhanexoft
Copy link
Contributor Author

ihorsokhanexoft commented Jun 23, 2025

A little more involved than I was expecting, but it looks like it should work. One question: does the draft registration inherit the custom storage allocation value from the node that it's being registered from? If not, it probably should.

Actually it's just two new fields in DraftRegistration and updating archiver logic to use these fields or default limits. More like refactoring of tests/admin functionality + new tests + fixing email templates to use new limits than actual implementation (still in discussion with Mark)

Regarding inheritance, no, it doesn't inherit limits from its node. It has been 5 GB for all registrations all this time regardless of custom storage limits of their node, so maybe we missed it in the ticket requirements. Should we do that now or it's better to ask product team about it?

@brianjgeiger
Copy link
Collaborator

Regarding inheritance, no, it doesn't inherit limits from its node. It has been 5 GB for all registrations all this time regardless of custom storage limits of their node, so maybe we missed it in the ticket requirements. Should we do that now or it's better to ask product team about it?

Since we're using the same fields that the node uses, rather than having a completely different setup for draft registrations, I personally think it makes sense to inherit the custom storage limits from the node. Please check with Product to see if they agree.

@ihorsokhanexoft
Copy link
Contributor Author

Regarding inheritance, no, it doesn't inherit limits from its node. It has been 5 GB for all registrations all this time regardless of custom storage limits of their node, so maybe we missed it in the ticket requirements. Should we do that now or it's better to ask product team about it?

Since we're using the same fields that the node uses, rather than having a completely different setup for draft registrations, I personally think it makes sense to inherit the custom storage limits from the node. Please check with Product to see if they agree.

Got the answer: No. We plan to separate projects and registrations in the near future. It's best if we prepare for that.

@ihorsokhanexoft
Copy link
Contributor Author

Got new requirements from product team. Draft registration should have only one limit that is applied regardless of the draft registration privacy
image

Copy link
Collaborator

@brianjgeiger brianjgeiger left a comment

Choose a reason for hiding this comment

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

Overall, looks good. Just one question about an edge case.

@ihorsokhanexoft
Copy link
Contributor Author

Admins can't set negative storage limits for draft registrations and nodes

Also I noticed another bug. If admins enter float limits (for nodes/draft registrations), admin templates display only the integer part of limits but these limits are saved correctly. Should I create a ticket in JIRA for it or spend some time on editing templates?

image
image

@brianjgeiger brianjgeiger changed the base branch from feature/pbs-25-10 to feature/pbs-25-13 June 30, 2025 15:10
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.

2 participants