Skip to content

Calculate Storage usage on node creation [ENG-2402]#9548

Open
corbinSanders wants to merge 3 commits intoCenterForOpenScience:developfrom
corbinSanders:feature/initialize_storage_usage
Open

Calculate Storage usage on node creation [ENG-2402]#9548
corbinSanders wants to merge 3 commits intoCenterForOpenScience:developfrom
corbinSanders:feature/initialize_storage_usage

Conversation

@corbinSanders
Copy link
Copy Markdown
Contributor

Purpose

We need to initialize storage usage on node creation. Whether this is a new node or a fork.

Changes

Adding update storage usage cache to save and forking methods. Adding tests

QA Notes

Please make verification statements inspired by your code and what your code touches.

  • Verify Storage usage is available after node creation.
  • Verify Storage usage is available and accurate after node forking.

What are the areas of risk?

Any concerns/considerations/questions that development raised? N/A

Documentation

N/A

Side Effects

Shouldn't be.

Ticket

https://openscience.atlassian.net/browse/ENG-2402

Copy link
Copy Markdown
Contributor

@Johnetordoff Johnetordoff left a comment

Choose a reason for hiding this comment

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

Looks pretty good, just have one question.

Comment thread osf/models/node.py
addon.after_fork(original, forked, user)

forked.save()
update_storage_usage_cache(forked.id, forked._id) # Files were copied in the addons. Need to update storage usage
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.

If the cache is updating on first save why is this necessary? Shouldn't it update the cache on the previous line?

Copy link
Copy Markdown
Contributor Author

@corbinSanders corbinSanders Dec 3, 2020

Choose a reason for hiding this comment

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

This isn't the first save. L1598 is the first save.

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.

Oh I see now, the hooks obviously aren't firing here, because it's just metadata.

Comment thread tests/test_addons.py Outdated
'contentType': 'img/png'
}).save()

update_storage_usage(self.node)
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.

nit: I'd move these into create version just to cut down on the number of new lines.

Copy link
Copy Markdown
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.

🎉 🐧 🎉

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.

3 participants