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

Set cache_ok flag to true for UUIDList #17232

Merged
merged 2 commits into from
Feb 21, 2025
Merged

Set cache_ok flag to true for UUIDList #17232

merged 2 commits into from
Feb 21, 2025

Conversation

cicdw
Copy link
Member

@cicdw cicdw commented Feb 21, 2025

Closes #17231

This PR sets the cache_ok flag to True for our custom UUIDList type in the sqlalchemy layer of the server. From the SA docs:

If the TypeDecorator is not guaranteed to produce the same bind/result behavior and SQL generation every time, this flag should be set to False; otherwise if the class produces the same behavior each time, it may be set to True. See TypeDecorator.cache_ok for further notes on how this works.

I'm still reading a little bit but this class should be fully deterministic, including preserving the order in which values are given, so this should be a strict improvement AFAIK.

@cicdw cicdw added the enhancement An improvement of an existing feature label Feb 21, 2025
@github-actions github-actions bot added the bug Something isn't working label Feb 21, 2025
@cicdw cicdw removed the bug Something isn't working label Feb 21, 2025
Copy link

codspeed-hq bot commented Feb 21, 2025

CodSpeed Performance Report

Merging #17232 will not alter performance

Comparing cache-ok (637bac9) with main (ba6d917)

Summary

✅ 2 untouched benchmarks

@cicdw cicdw merged commit 29748d5 into main Feb 21, 2025
46 checks passed
@cicdw cicdw deleted the cache-ok branch February 21, 2025 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Performance warning with UUIDList() and SQLAlchemy for a self-hosted server
3 participants