-
Notifications
You must be signed in to change notification settings - Fork 31
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
Upgrade Windshaft #3109
Upgrade Windshaft #3109
Conversation
This comment has been minimized.
This comment has been minimized.
0f849f6
to
f2a1ed5
Compare
Scope across `app` and `tiler` virtual machines.
Helps combat what appear to be race conditions during the `npm install` over vboxsf.
b98add3
to
8fa0dce
Compare
Previously, the health-check (which was based off old examples) used transitive dependencies in its `require` statements. Change those to direct dependencies, as is the standard. Also add some dependencies to be used in the upcoming upgrade.
Previously Windshaft included a built-in server. That must now be specified explicitly, as done in OpenTreeMap/otm-tiler#116, and as shown in their examples https://github.com/CartoDB/Windshaft/tree/be55cb931a157be3e7da9e9c810929f48c342789/examples The beforeTileRender and afterTileRender hooks are gone, so that functionality is moved in to the server code. Some of this code has been updated / adapted from the examples to make more sense, and be less redundant. Other parts remain from the examples. It could use a proper overhaul.
Apparently subqueries should not have parens anymore
Previously, we had a single style file that was used for every query. The layer name tags in the style were used to pick what is applied to which layer. Unfortunately, that is no longer supported, and we must only specify the relevant styles for each layer in the call. This requires separating the styles into separate files so they don't interfere with one another. Also, we can no longer specify a table name and have it generate the appropriate SQL. The query must be specified explicitly.
8fa0dce
to
4dbcc7b
Compare
4dbcc7b
to
5fc993c
Compare
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.
Currently, I can get this to provision cleanly on Status when trying to start the service manually:
When using
I'll try to do some additional debugging. |
Resolved the above by re-provisioning tiler. There were indications that the error was typical of corrupted |
Probably should've included a note in the testing instructions to make sure your host's |
I was hitting the same `bundle.sh` failures others were hitting this morning.
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.
This is working great. Upgrade applied cleanly and all tiles are rendering with appropriate styles. The TN/TSS/TP DRB streams are just a fraction thicker than on the main site, but I can't find a change to the line width and is too subtle to notice. Really great job working through this difficult upgrade.
The line widths are all multiples of model-my-watershed/src/tiler/styles.mss Lines 195 to 208 in e70fef2
which was previously defined as model-my-watershed/src/tiler/styles.mss Line 134 in e70fef2
In my testing the upgraded version was rendering thinner lines, so I upped it to
Feel free to play around with that value and suggest a better one. |
I think it's completely within tolerance given all the rendering upgrades that happened. |
Thanks for the thorough review and all the support! Merging this now. |
Overview
Upgrades the Windshaft tiler.
Connects #3091
Connects #3082
Demo
Notes
Currently getting the following error in the "Install Windshaft JavaScript Dependencies" step:fixed in 8c5d49a.Rollbar and S3 Caching will be tested once this is on staging.
Testing Instructions
Delete the
src/tiler/node_modules
directory in your hostCheck out this branch, destroy the
tiler
and reprovision itEnsure you have various stream / boundary datasets installed:
$ vagrant ssh app -c 'cd /vagrant && ./scripts/aws/setupdb.sh -b -d -s -c -p -q -m'
Go to :8000/ and turn on each layer in the layer selector
Choose "Select Boundary" as the draw tool, and pick HUC-8 as the boundary type
Hover around an area which is homogenous, i.e. has the same boundary value for entire tiles
Search away from that area to another homogenous area, i.e. has a different, but same boundary value for entire tiles