-
Notifications
You must be signed in to change notification settings - Fork 9
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
Exit mbtiles read stream if we hit a tile maximum #29
base: master
Are you sure you want to change the base?
Conversation
This is ready to go! Added a test that uses a known fixture of four tiles with four unique property values and only asks for a sample of those tiles, asserting that the number of unique property values is equal to the number of tiles requested. Would appreciate a review from anyone if they have time! |
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.
👍 !
@@ -408,3 +408,23 @@ test('MBTiles with two layers', t => { | |||
t.end(); | |||
}).catch(t.threw); | |||
}); | |||
|
|||
test('MBTiles limits max tiles', t => { |
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.
👏 Wow, great test and fixture.
readStream.on('data', function() { | ||
if (count >= options.maxTiles) { | ||
readStream.unpipe(analysisStream); | ||
readStream.end(); |
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.
Nice. Properly closing streams is typically a big 👀 ❓ for me , but this looks super clear and👌
Throw a wrench at the mbtiles readstream if we hit a reasonable tile count.
refs: #30
cc @rclark @GretaCB