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

Remove layer filter #680

Merged
merged 1 commit into from
Jan 29, 2019
Merged

Remove layer filter #680

merged 1 commit into from
Jan 29, 2019

Conversation

KlaasH
Copy link
Contributor

@KlaasH KlaasH commented Jan 29, 2019

Overview

Removes the querystring-based layer filter functionality, since it's not being used and isn't compatible with the intended caching strategy.

Notes

  • The immediate impetus for removing it (rather than letting it sit there, which it was doing before without hurting anything) is that it turns out claudia-local-api and the actual API Gateway proxy's request objects are different in the case of a request with no querystring--claudia-local-api provides an empty object, but API Gateway sets req.queryStringParameters to null. This caused a crash on Lambda that wasn't happening locally. And yeah, any deviation from "just sitting there not getting in the way" is cause enough to rip this out.
  • This takes the implementation of Tilegarden here farther from the upstream Tilegarden project, but this implementation was always meant to be customized, and it's already far enough from upstream that there's no question of merging or large-scale copying in either direction. Which is fine.

Testing Instructions

This removes some tests, since there was a whole test file for the layer filtering. Since the issue that prompted this only showed up on AWS, local testing only goes so far. It makes sense to push this to staging manually and confirm that the analysis tiles show up. Which can be accomplished by pulling down the .env and claudia.json from the Staging settings bucket like in scripts/infra and running the Tilegarden deployment command (docker-compose -f docker-compose.yml -f docker-compose.test.yml run --rm --entrypoint yarn tilegarden deploy).

Connects #634 and #635

Removes the querystring-based layer filter functionality, since it's not
being used and isn't compatible with the intended caching strategy.
Copy link
Contributor

@flibbertigibbet flibbertigibbet left a comment

Choose a reason for hiding this comment

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

I don't have context for this as I haven't worked on Tilegarden, and can't really test this, but it builds and looks fine 👍

@KlaasH KlaasH merged commit a7daa2a into develop Jan 29, 2019
@KlaasH KlaasH deleted the feature/kjh/remove-layer-filter branch January 29, 2019 17:14
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