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

Copy tilesource instances to add styles #894

Merged
merged 2 commits into from
Aug 5, 2022

Conversation

manthey
Copy link
Member

@manthey manthey commented Jul 25, 2022

Instead of reopening a tilesource when adding a style, copy the unstyled class and add the style information.

I've marked this as draft as I think it requires more tests to verify it does no harm.

@manthey
Copy link
Member Author

manthey commented Jul 25, 2022

@banesullivan This should speed up generating regions and tiles with different styles from the same source file by avoiding reopening the file handles. I think it should work without issue, but, since file handles and locks are shared across classes, I think there is a potential that some additional lock may be needed in some case.

Please try it out and let me know if you see any issues when using multiple styles and sources. I'll convert it to a non-draft when I've added more tests.

@banesullivan
Copy link
Contributor

Thanks for pinging me on this. I'd like to stress test this with Django-large-image when I get back from PTO (tomorrow)

manthey added 2 commits August 4, 2022 16:07
Instead of reopening a tilesource when adding a style, copy the unstyled
class and add the style information.
@manthey manthey force-pushed the copy-styled-tilesource-class branch from 589b66e to 7c60918 Compare August 4, 2022 20:08
@manthey manthey marked this pull request as ready for review August 4, 2022 20:28
@manthey
Copy link
Member Author

manthey commented Aug 4, 2022

I've added some tests to ensure sources don't close handles prematurely. @banesullivan If your testing is successful, I think we can merge this.

@banesullivan
Copy link
Contributor

LGTM. Tests in django-large-image are happy, and my poking and prodding went well

@manthey manthey merged commit 28766fd into master Aug 5, 2022
@manthey manthey deleted the copy-styled-tilesource-class branch August 5, 2022 12:03
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