Write tiles to S3 cache #673
Conversation
There was a problem hiding this comment.
Looks great to me! I was able to push tiles up in dev, and also remove the env var to block the upload. The clear script worked fine, too. Seems like there are a few linter errors in the Jenkins log, but I trust you'll fix those.
Good work reconstructing the path from parameters, that looks like it was slightly tricky.
scripts/clear-tile-cache
Outdated
|
|
||
| set -e | ||
|
|
||
| BUCKET_NAME=$(docker-compose run --rm tilegarden bash -c 'echo -n $PFB_TILEGARDEN_CACHE_BUCKET') |
There was a problem hiding this comment.
Do you think it's worth allowing the user to set this bucket themselves, without having to edit common.yml? e.g.:
if [[ -z "${BUCKET_NAME}" ]]; then
BUCKET_NAME=$(docker-compose run --rm tilegarden bash -c 'echo -n $PFB_TILEGARDEN_CACHE_BUCKET')
fiI agree with your comment in the PR body that it's hard to think of cases where this script would be useful other than clearing the preconfigured dev tile cache, so I leave the decision up to you.
There was a problem hiding this comment.
Yeah, on reflection that's probably worth having. It might not be useful, but then again it might, in which case it's a lot easier to write it in now than muddle through without it later.
|
Whoops, I guess I rushed this out the door a bit. Will clean up linting and tests. |
| @@ -18,6 +18,7 @@ services: | |||
| - PFB_AWS_BATCH_TILEMAKER_JOB_DEFINITION_NAME_REVISION | |||
| - PFB_AWS_BATCH_TILEMAKER_JOB_DEFINITION_NAME | |||
| - PFB_TILEGARDEN_ROOT=http://localhost:9400 | |||
| - PFB_TILEGARDEN_CACHE_BUCKET=dev-pfb-tilecache-us-east-1 | |||
There was a problem hiding this comment.
Why not put the tiles for development users somewhere in the existing ${DEV_USER}-pfb-storage-us-east-1 bucket we're using for other S3 storage?
There was a problem hiding this comment.
I'm expecting that for serving tiles from the cache we'll want to use an S3 Website, since that's what I did in the Tilegarden branch where I've been working on caching. And as far as I can tell that serves the whole bucket. If there turns out to be a way to restrict it and use the existing bucket, that would be worth pursuing.
It's true that a shared dev cache could cause some inconvenience, though given that tiles are namespaced by job ID, it would be more in the way of clutter than any weird tiler behavior. I'm inclined to run with it, though--with the fallback of making a personal bucket and overriding the variable if one is needed--until we know whether it's a problem in practice.
See https://claudiajs.com/news/2016/09/27/claudia-2.0.html#request The `req` object we've been using is described as "for backwards compatibility" and it seems like the AWS_PROXY one is potentially more robust and portable.
Adds a function to write a tile to S3 if a `PFB_TILEGARDEN_CACHE_BUCKET` variable is set in the environment. If the variable isn't truthy, it does nothing. Uses the full request path as the key.
KlaasH
force-pushed
the
feature/kjh/write-tiles-to-cache
branch
from
d64edd6 to
a402de0
Compare
|
Linter appeased, bucket argument added to |
Overview
Adds a step in the tile generation process where it checks for a
PFB_TILEGARDEN_CACHE_BUCKETvariable in the environment and, if it's set, writes the tile into that bucket, using the request path as the key.Currently there's no way to serve the tiles from cache. #634 covers that part.
Also adds a script to clear all files in the configured cache bucket. I'm not sure that will be the way to do it in deployed environments (where it's harder to get a shell, and where the number of items might make the "change settings to make all objects expire, change settings back" deletion strategy preferable), but it seems useful in development.
Demo
Notes
AWS_PROXYrequest format for Claudia API Builder. See docs.claudia-local-apidoesn't fully reproduce the AWS Proxy-style request object (see Support AWS_PROXY request format claudia-local-api#1). As noted in the code, this works around that by reconstructing the path from the parameters.Testing Instructions
./scripts/updateand./scripts/serverdev-pfb-tilecache-us-east-1bucket (you can run./scripts/clear-tile-cacheto clear out the clutter if it's hard to tell).Resolves #635
Resolves #636