-
-
Notifications
You must be signed in to change notification settings - Fork 118
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
Support unzipping GeoPackage sources at runtime #430
Conversation
https://github.com/onthegomap/planetiler/actions/runs/4011565777 ℹ️ Base Logs ae1317c
ℹ️ This Branch Logs bc9f4f7
|
planetiler-core/src/main/java/com/onthegomap/planetiler/reader/GeoPackageReader.java
Outdated
Show resolved
Hide resolved
planetiler-core/src/main/java/com/onthegomap/planetiler/Planetiler.java
Outdated
Show resolved
Hide resolved
planetiler-core/src/main/java/com/onthegomap/planetiler/util/FileUtils.java
Outdated
Show resolved
Hide resolved
planetiler-core/src/main/java/com/onthegomap/planetiler/util/FileUtils.java
Outdated
Show resolved
Hide resolved
Hey @erik just checking in on this, were you planning to make more changes? |
@msbarry yeah, hopefully will have a chance this upcoming week, holidays have kept me busy. Couple of questions for you in the comments above which might reduce number of review iterations |
Hey @msbarry, just making sure this didn't slip through the cracks, any other changes you'd like to see here? |
Sorry for the delay! Looks good, just have a couple of minor tweaks to the tests, I'll try pushing them then land. One thing I noticed is that it's 2-3x slower than natural earth reader for the natural earth geopackage file. Not a big deal in the grand scheme of things since it only takes ~30 seconds no matter the size of the extract, I'm just curious if there's any obvious reason for the slowdown... |
@msbarry when I was first working on this I believe I noticed that the |
Kudos, SonarCloud Quality Gate passed! |
OK got it, that makes sense. One other thing is that it now unzips the zip file twice - once to get the count, then again to read the file. Another option would be send the unzipped file to a deterministic location, and keep it around between those. @bdon also pointed out that we could keep it around between runs to save from having to unzip it each time. |
Extension to #413 which allows us to read GeoPackage sources from ZIP archives.
Testing locally, I was able to use the NaturalEarth data from https://naciscdn.org/naturalearth/packages/natural_earth_vector.gpkg.zip as a direct replacement for
addNaturalEarthSource()
.We should be able to remove
NaturalEarthReader
, but since there are several references tonatural_earth_vector.sqlite.zip
which will break, I've left it as a@Deprecated
annotation for now. At the very least I think we'd need to update theplanetiler-openmaptiles
repo first to keep tests from breaking.