-
Notifications
You must be signed in to change notification settings - Fork 4
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
GTC-2888: Allow specifying tippecanoe feature filters #556
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #556 +/- ##
===========================================
- Coverage 81.92% 81.92% -0.01%
===========================================
Files 126 126
Lines 5688 5692 +4
===========================================
+ Hits 4660 4663 +3
- Misses 1028 1029 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
NDJSON_FILE="data.json" | ||
|
||
echo "Fetching NDJSON data from the Data Lake: ${SRC} -> ${NDJSON_FILE}..." | ||
aws s3 cp "${SRC}" "${NDJSON_FILE}" --no-progress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit: is there a reason you moved the copy here earlier in the script. If not, I would move it back to where it was, since you don't want to do the copy if we fail in the case statement below with a bad tile cache option. (I know that is not likely to happen, because other code is generating this command.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point, the reason for the move is lost to the sands of git. I will move it back, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dmannarino and I walked through this together during a pairing session.
Pull request checklist
Please check if your PR fulfills the following requirements:
Pull request type
Please check the type of change your PR introduces:
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this introduce a breaking change?
Other information
Proposed request body:
{ "asset_type": "Static vector tile cache", "creation_options": { "min_zoom": 0, "max_zoom": 9, "implementation": "attempt9", "tile_strategy": "discontinuous", "layer_style": [ { "id": "gadm_administrative_boundaries", "type": "fill", "source": "gadm_administrative_boundaries", "source-layer": "gadm_administrative_boundaries" } ], "feature_filter": { "*": [ "any", ["all", [">=", "$zoom", 0], ["==", "adm_level", "0"] ], ["all", [">=", "$zoom", 3], ["==", "adm_level", "1"] ], ["all", [">=", "$zoom", 7], ["==", "adm_level", "2"] ] ] } } }