[WIP] Allow throwing error on max vtile size + logging large tiles #92
Conversation
springmeyer
changed the title
…_VTILE_BYTES_COMPRESSED is defined
| stats.max = pbfz.length; | ||
| } | ||
| } | ||
| if (source.BRIDGE_MAX_VTILE_BYTES_COMPRESSED > 0 && pbfz.length > source.BRIDGE_MAX_VTILE_BYTES_COMPRESSED) { |
There was a problem hiding this comment.
@springmeyer do we want to error here right away? Perhaps we can comment this out for now until we are ready to return an error.
There was a problem hiding this comment.
@mapsam - totally okay to comment, however this is harmless currently because it is a different environment variable. Note BRIDGE_MAX_VTILE_BYTES_COMPRESSED vs BRIDGE_LOG_MAX_VTILE_BYTES_COMPRESSED
There was a problem hiding this comment.
ah, I didn't notice that. Thanks for pointing it out - keeping it here works for me!
index.js
Outdated
|
|
||
| process.on('exit',function() { | ||
| stats.avg = stats.total/stats.count; | ||
| fs.writeFileSync('tilelive-bridge-stats.json',JSON.stringify(stats,null,1)); |
There was a problem hiding this comment.
Confirming the location this will write to - it be where the function was executed from, rather than within this module, correct? @springmeyer
There was a problem hiding this comment.
Right, not going to write into the module path. That will currently log to the current working directory. So the directory of the user running mapbox-tile-copy.
index.js
Outdated
| @@ -8,10 +8,21 @@ var immediate = global.setImmediate || process.nextTick; | |||
| var mapnik_pool = require('mapnik-pool'); | |||
| var Pool = mapnik_pool.Pool; | |||
| var os = require('os'); | |||
| var bytes = require('bytes'); | |||
There was a problem hiding this comment.
looks like we aren't using this any longer @springmeyer
There was a problem hiding this comment.
yes, please feel free to remove.
|
Just left a few comments @springmeyer! Not sure why some of the tests are failing, perhaps just a blip? |
Looks like the rendering benchmarks are failing - either a blip or a legit slowdown. 🤔 |
I can't pick up a meaningful difference in speeds locally with and without this branch, so it may be on travis's end. Running benchmarks on travis is dubious. |
It is, looks like these have been failing in master for some time: https://travis-ci.org/mapbox/tilelive-bridge/jobs/242849687#L1344 |
|
@springmeyer happy to let them continue to fail here or remove assertions on benchmarks from tests. Your call! |
|
@springmeyer myself and @GretaCB were working with this and noticed by using |
|
@springmeyer @GretaCB I just updated the README to reflect the environment variable changes. Will plan on merging this in and releasing as 2.5.0. |
Adds the ability to:
Downstream consumers can opt into either of these options.
In the case of the max size error, the buffer is still passed in the callback. This allows downstream consumers to decide on the logic they want to use when this happens.