Skip to content
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

[ENG-6801] Preprint _id caching #10881

Conversation

Ostap-Zherebetskyi
Copy link

Purpose

Make _id a cached property

Changes

see diff

QA Notes

N/A

Documentation

N/A

Side Effects

N/A

Ticket

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

osf/models/base.py Outdated Show resolved Hide resolved
@Ostap-Zherebetskyi Ostap-Zherebetskyi marked this pull request as ready for review January 2, 2025 14:07
Copy link
Collaborator

@cslzchen cslzchen left a comment

Choose a reason for hiding this comment

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

Our code base is confusing ... most newer pieces use Django's built-in @cached_property while this old Guid piece still uses our own implementation of @cached_property ... and I also found another ancient flask model using @cached_property fromwerkzeug ...

Screenshot 2025-01-03 at 16 13 55

Can you take a look at if django.utils.functional.cached_property is worth switching and if there is any compatibility concerns? @mfraezz will be back on Monday so we can ask for his input too.

osf/models/preprint.py Outdated Show resolved Hide resolved
osf/models/preprint.py Outdated Show resolved Hide resolved
osf/models/preprint.py Outdated Show resolved Hide resolved
osf_tests/management_commands/test_check_crossref_dois.py Outdated Show resolved Hide resolved
osf_tests/test_guid.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@cslzchen cslzchen left a comment

Choose a reason for hiding this comment

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

Looks good and will merge to a different feature branch that is not on staging2.

@cslzchen
Copy link
Collaborator

cslzchen commented Jan 7, 2025

Cherry-picked into #10892 and will merge there

osf/utils/caching.py Show resolved Hide resolved
osf/utils/caching.py Show resolved Hide resolved
@cslzchen
Copy link
Collaborator

cslzchen commented Jan 7, 2025

Merged in 7b16120

@cslzchen cslzchen closed this Jan 7, 2025
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