add option to valhalla_build_extract to use existing tar file for lookup/copy #4255
Conversation
…xisting extract tar
| @@ -53,6 +53,38 @@ class TileHeader(ctypes.Structure): | |||
| ] | |||
|
|
|||
|
|
|||
| class TileResolver: | |||
There was a problem hiding this comment.
a wrapper so we don't have to care about whether we're pulling tiles from a dir or tar
There was a problem hiding this comment.
I don't really like this construct. Usually I'd do a base class and 2 inherited ones, but for brevity's sake I did just this.
scripts/valhalla_build_extract
Outdated
| parser.add_argument( | ||
| "-O", | ||
| "--overwrite", | ||
| help="Overwrites an output tar file", | ||
| action="store_true" | ||
| ) |
There was a problem hiding this comment.
this has nothing to do with the actual PR's changes, it's just smth we should do as small protection
scripts/valhalla_build_extract
Outdated
| parser.add_argument( | ||
| "-e", | ||
| "--extract-tar", | ||
| help="If specified, will build an extract from an existing tar file at 'mjolnir.tile_extract' and save it to this specified path.", | ||
| type=str, | ||
| default="" | ||
| ) |
There was a problem hiding this comment.
so if someone specifies e.g. -e new_tar.tar, we'd write the result to ./new_tar.tar instead of what specified in the config. the tar specified in the config is instead taken as input.
| tiles_dir: Path = Path(config["mjolnir"].get("tile_dir", '/dev/null')) | ||
| if not tiles_dir.is_dir(): | ||
| # optionally use the tile extract to find the tiles | ||
| if args.extract_tar and (tiles_extract_in := Path(config["mjolnir"].get("tile_extract"))).is_file(): |
There was a problem hiding this comment.
first time ever I used the cute "walrus" operator haha it's python3.8 (3.7 is EOL by now)
|
There's still some small stuff failing, will look at it tmrw. |
|
For future reference, I should note how this should be used: If you want to use |
|
|
||
|
|
||
| class TestBuildExtract(unittest.TestCase): | ||
| def test_tile_intersects_bbox(self): | ||
| # bogus tile dir | ||
| tile_dir = Path("/home/") | ||
| tile_dir = Path("/foo/") |
There was a problem hiding this comment.
/home is far away from a "bogus dir" haha
| @@ -173,11 +223,9 @@ def get_tiles_with_bbox(all_tile_paths: List[Path], bbox_str: str, tiles_dir_: P | |||
| LOGGER.critical(f"Bbox invalid: {list(bbox)}") | |||
| sys.exit(1) | |||
|
|
|||
| tile_paths_ = set() | |||
There was a problem hiding this comment.
this used to collect a set before. in python, that's an unordered structure, so really we were producing unordered tar files 🙄 my bad.. it's a list now and we deduplicate just before writing it to the tar
There was a problem hiding this comment.
for reference: we can get duplicate tiles for --geojson-dir where multiple polygons intersect the same tile.
nilsnolde
force-pushed
the
nn-extract-from-tar
branch
from
64af516
to
48f8ab3
Compare
|
can you re-approve again pls @kevinkreiser ? I added a changelog, so it dimissed. |
When running an instance of Valhalla, what one usually has is the tar file, and often no plain tile directory. Then
valhalla_build_extractcan't be used, which is a pity. At least for the public instance, I regularly have that "problem" and I don't want to un-tar those 80 GB before I can rip a few out.This PR adds support to build extracts from tar files.