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

Caching at higher levels than specified by maxZoom #311

Closed
ear7h opened this issue Feb 19, 2018 · 3 comments
Closed

Caching at higher levels than specified by maxZoom #311

ear7h opened this issue Feb 19, 2018 · 3 comments
Labels
Milestone

Comments

@ear7h
Copy link
Contributor

ear7h commented Feb 19, 2018

From what I understand here higher zooms correspond with higher numbers. However the code in the filecache and s3cache packages assume the opposite, not caching when the zoom level is lower than MaxZoom.

// inside Set() method

//	check for maxzoom
if fc.MaxZoom != nil && key.Z <= int(*fc.MaxZoom) {
	return nil
}

// do cache

Also, I think mechanism is makes odd use of pointers and uint types. I'm willing to change this but wanted to submit an issue for clarification.

@ARolek
Copy link
Member

ARolek commented Feb 19, 2018

@ear7h you're correct about "lower zooms" being closer to zero, and this looks like a bug. Thanks for reporting it. I will add a test case to both the filecache and s3cache and get this fixed.

Regarding the uint / pointer piece being odd, I agree. This is because some of the typing is going through a transition. Tile requests come to tegola in the format of /:map_name/:z/:x/:y. Currently we parse the z/x/y values into int values, but then we hit a bug with invalid requests of negative values. You can see the checking happening here. The plan is to convert everything to uint values and drop the less than zero checking. This will also require an update to the cache interface and then some code cleanup to the various cache implementations. I will open another issue to track it.

@ARolek ARolek added the bug label Feb 19, 2018
@ARolek ARolek added this to the v0.6.0 milestone Feb 19, 2018
@ear7h
Copy link
Contributor Author

ear7h commented Feb 19, 2018

Okay sounds good. I'll look for those tests when you submit them and make corresponding ones for the rediscache

ARolek added a commit that referenced this issue Feb 21, 2018
- added test cases for max_zoom
- removed package stutter s3cache -> s3
- removed package stutter filecache -> file
- updated test cases to use new convetion
@ARolek
Copy link
Member

ARolek commented Feb 21, 2018

@ear7h my recent PR (#314) has the max_zoom fix and associated tests. Thanks again for the report. Teach me to not write test cases... ;-)

gdey added a commit that referenced this issue Feb 21, 2018
max_zoom bug on s3 and file caches #311
@ARolek ARolek closed this as completed Feb 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants