diff --git a/.github/workflows/on-push.yml b/.github/workflows/on-push.yml index 729efbc..6e64348 100644 --- a/.github/workflows/on-push.yml +++ b/.github/workflows/on-push.yml @@ -44,7 +44,7 @@ jobs: python-version: ${{ matrix.python-version }} - name: Install the GDAL command line tools - run: sudo apt-get install -y gdal-bin + run: sudo apt-get update && sudo apt-get install -y gdal-bin - name: Run unit tests run: make unit-tests COV_REPORT=xml @@ -121,7 +121,7 @@ jobs: python-version: '3.14' - name: Install the GDAL command line tools - run: sudo apt-get install -y gdal-bin + run: sudo apt-get update && sudo apt-get install -y gdal-bin - name: Run tests run: make minver-tests diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1e0afe1..0813ecd 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -9,6 +9,7 @@ built with ``gdalbuildvrt``. - Drop the ``make``, ``curl``, ``unzip`` and ``gunzip`` dependencies: tile downloads are now sequential. +- Make caching the tiles missing upstream, e.g. ocean tiles, more robust - Drop the ``reference`` optional dependencies: the bounds of a reference data source are read with the ``gdalinfo`` and ``ogrinfo`` command line tools. - Add the ``--gdal-options`` option and the ``EIO_CLIP_GDAL_OPTIONS`` variable to pass diff --git a/elevation/cache.py b/elevation/cache.py index 526c3d3..218bc36 100644 --- a/elevation/cache.py +++ b/elevation/cache.py @@ -68,9 +68,8 @@ def lock_tiles(datasource_root: Path, tile_names: list[str]) -> Generator[None]: def is_cached(root: Path, tile_name: str) -> bool: - """Return whether *tile_name* is cached, i.e. present and not empty.""" - cached = root / "cache" / (tile_name + ".tif") - return cached.exists() and cached.stat().st_size > 0 + """Return whether *tile_name* is cached, empty tiles included.""" + return (root / "cache" / (tile_name + ".tif")).exists() def ensure_tiles( @@ -78,6 +77,7 @@ def ensure_tiles( tiles: list[Tile], prepare_tile: Callable[..., str], gdal_options: str = spatial.INT_TILE_GDAL_OPTIONS, + empty_on_notfound: bool = True, **kwargs: Any, ) -> None: """Fetch and cache *tiles*, skipping the tiles already in the cache. @@ -93,7 +93,9 @@ def ensure_tiles( # convert the data to the internal cache format spool = root / "spool" / (tile_name + ".tif") - spatial.call_gdal_translate(source, spool, options=gdal_options) + spatial.call_gdal_translate( + source, spool, options=gdal_options, empty_on_notfound=empty_on_notfound + ) # finally move the data inside the cache. The move is atomic in most cases cached = root / "cache" / (tile_name + ".tif") diff --git a/elevation/datasource.py b/elevation/datasource.py index 969f24b..5b4f74f 100644 --- a/elevation/datasource.py +++ b/elevation/datasource.py @@ -167,6 +167,7 @@ class DatasourceSpec(TypedDict): prepare_tile: Callable[..., str] prepare_tile_kwargs: dict[str, Any] tile_gdal_options: NotRequired[str] + empty_on_notfound: NotRequired[bool] MAPZEN_SPEC: DatasourceSpec = { @@ -210,6 +211,7 @@ class DatasourceSpec(TypedDict): "chunks": (3600, 1800), }, "tile_gdal_options": spatial.FLOAT_TILE_GDAL_OPTIONS, + "empty_on_notfound": False, "cached_tiles": zarr_tiles, "cached_tiles_kwargs": {"transform": EDH_L2_CHUNK_INDECES_TRANSFORM}, } @@ -221,6 +223,7 @@ class DatasourceSpec(TypedDict): "chunks": (2400, 2400), }, "tile_gdal_options": spatial.FLOAT_TILE_GDAL_OPTIONS, + "empty_on_notfound": False, "cached_tiles": zarr_tiles, "cached_tiles_kwargs": {"transform": EDH_L1_CHUNK_INDECES_TRANSFORM}, } @@ -314,6 +317,7 @@ def seed( downloads, prepare_tile=prepare_tile, gdal_options=spec.get("tile_gdal_options", spatial.INT_TILE_GDAL_OPTIONS), + empty_on_notfound=spec.get("empty_on_notfound", True), **prepare_tile_kwargs, ) diff --git a/elevation/spatial.py b/elevation/spatial.py index 66290ff..7792bcf 100644 --- a/elevation/spatial.py +++ b/elevation/spatial.py @@ -14,7 +14,10 @@ # limitations under the License. import json +import re import subprocess +import urllib.error +import urllib.request from pathlib import Path from typing import Any @@ -25,6 +28,8 @@ ) INT_TILE_GDAL_OPTIONS = TILE_GDAL_OPTIONS + " -co PREDICTOR=2" FLOAT_TILE_GDAL_OPTIONS = TILE_GDAL_OPTIONS + " -co PREDICTOR=3" +# the remote file of a /vsizip chain is the archive, not the member +VSICURL_PATTERN = re.compile(r"/vsicurl/([^\s\"]+?\.zip(?=/)|[^\s\"]+)") TOOLS: list[tuple[str, str]] = [ ("gdal_translate", "gdal_translate --version"), ("gdalbuildvrt", "gdalbuildvrt --version"), @@ -62,16 +67,28 @@ def gdal_json(cmd: str, destination: str) -> Any: return json.loads(output) +def is_not_found(url: str) -> bool: + """Return whether the HTTP server answers 404 for *url*.""" + request = urllib.request.Request(url, method="HEAD") + try: + with urllib.request.urlopen(request, timeout=60): + return False + except urllib.error.HTTPError as error: + return error.code == 404 + + def call_gdal_translate( source: str, destination: Path, options: str = DEFAULT_GDAL_OPTIONS, + empty_on_notfound: bool = False, ) -> list[str]: """Write *source* to *destination* calling the gdal_translate binary. :param source: Any GDAL readable raster, local or remote, may include selection options. :param destination: Path of the destination, parent folders are created. :param options: GDAL creation options, the default is good for caching tiles. + :param empty_on_notfound: Write an empty *destination* if the remote *source* is a 404. """ destination.parent.mkdir(parents=True, exist_ok=True) cmd = [ @@ -80,7 +97,13 @@ def call_gdal_translate( *source.split(), str(destination), ] - subprocess.check_call(cmd) + try: + subprocess.check_call(cmd) + except subprocess.CalledProcessError: + url = VSICURL_PATTERN.search(source) + if not (empty_on_notfound and url and is_not_found(url[1])): + raise + destination.write_bytes(b"") return cmd diff --git a/tests/data/SRTM1_ELLIP/se_tasman_sea.tif b/tests/data/SRTM1_ELLIP/se_tasman_sea.tif new file mode 100644 index 0000000..806f5b6 Binary files /dev/null and b/tests/data/SRTM1_ELLIP/se_tasman_sea.tif differ diff --git a/tests/data/SRTM1_GEOID/se_tasman_sea.tif b/tests/data/SRTM1_GEOID/se_tasman_sea.tif new file mode 100644 index 0000000..c476716 Binary files /dev/null and b/tests/data/SRTM1_GEOID/se_tasman_sea.tif differ diff --git a/tests/data/SRTM3/nw_pacific.tif b/tests/data/SRTM3/nw_pacific.tif new file mode 100644 index 0000000..a87ce10 Binary files /dev/null and b/tests/data/SRTM3/nw_pacific.tif differ diff --git a/tests/integration_srtm1_ellip.py b/tests/integration_srtm1_ellip.py index 11974c6..8c6cc38 100644 --- a/tests/integration_srtm1_ellip.py +++ b/tests/integration_srtm1_ellip.py @@ -41,3 +41,9 @@ def test_se_sydney(integration_data: IntegrationData) -> None: def test_sw_santiago(integration_data: IntegrationData) -> None: bounds = (-70.6, -33.42, -70.6 + SIZE, -33.42 + SIZE) integration_data("SRTM1_ELLIP", "sw_santiago", bounds) + + +def test_se_tasman_sea(integration_data: IntegrationData) -> None: + # the S34E152 ocean tile is missing upstream + bounds = (152 - SIZE / 2, -33.9, 152 + SIZE / 2, -33.9 + SIZE) + integration_data("SRTM1_ELLIP", "se_tasman_sea", bounds) diff --git a/tests/integration_srtm1_geoid.py b/tests/integration_srtm1_geoid.py index b5bf5d7..649bd44 100644 --- a/tests/integration_srtm1_geoid.py +++ b/tests/integration_srtm1_geoid.py @@ -33,3 +33,9 @@ def test_se_sydney(integration_data: IntegrationData) -> None: def test_sw_santiago(integration_data: IntegrationData) -> None: bounds = (-70.6, -33.42, -70.6 + SIZE, -33.42 + SIZE) integration_data("SRTM1_GEOID", "sw_santiago", bounds) + + +def test_se_tasman_sea(integration_data: IntegrationData) -> None: + # the S34E152 ocean tile is missing upstream + bounds = (152 - SIZE / 2, -33.9, 152 + SIZE / 2, -33.9 + SIZE) + integration_data("SRTM1_GEOID", "se_tasman_sea", bounds) diff --git a/tests/integration_srtm3.py b/tests/integration_srtm3.py index 80dfa6c..a4d8266 100644 --- a/tests/integration_srtm3.py +++ b/tests/integration_srtm3.py @@ -33,3 +33,9 @@ def test_se_sydney(integration_data: IntegrationData) -> None: def test_sw_santiago(integration_data: IntegrationData) -> None: bounds = (-70.6, -33.42, -70.6 + SIZE, -33.42 + SIZE) integration_data("SRTM3", "sw_santiago", bounds) + + +def test_nw_pacific(integration_data: IntegrationData) -> None: + # the srtm_11_05 ocean tile is missing upstream + bounds = (-125 - SIZE / 2, 37.74, -125 + SIZE / 2, 37.74 + SIZE) + integration_data("SRTM3", "nw_pacific", bounds) diff --git a/tests/test_10_spatial.py b/tests/test_10_spatial.py index 83db740..369a944 100644 --- a/tests/test_10_spatial.py +++ b/tests/test_10_spatial.py @@ -2,6 +2,9 @@ # Copyright (c) 2016-2026 B-Open Solutions srl - https://bopen.eu # +import subprocess +import urllib.error +from email.message import Message from pathlib import Path import pytest @@ -59,6 +62,66 @@ def test_call_gdal_translate_command(tmp_path: Path, mocker: MockerFixture) -> N assert destination.parent.is_dir() +def test_is_not_found(mocker: MockerFixture) -> None: + url = "https://example.com/N41E012.tif" + urlopen = mocker.patch("urllib.request.urlopen") + assert not spatial.is_not_found(url) + assert urlopen.call_args.args[0].full_url == url + assert urlopen.call_args.args[0].get_method() == "HEAD" + + urlopen.side_effect = urllib.error.HTTPError(url, 404, "Not Found", Message(), None) + assert spatial.is_not_found(url) + + urlopen.side_effect = urllib.error.HTTPError( + url, 401, "Unauthorized", Message(), None + ) + assert not spatial.is_not_found(url) + + +@pytest.mark.parametrize( + "source,url", + [ + ("/vsicurl/https://h/N41E012.tif", "https://h/N41E012.tif"), + ("/vsigzip//vsicurl/https://h/N41E012.hgt.gz", "https://h/N41E012.hgt.gz"), + ( + "/vsizip//vsicurl/https://h/srtm_39_04.zip/srtm_39_04.tif", + "https://h/srtm_39_04.zip", + ), + ], +) +def test_call_gdal_translate_empty_on_notfound( + tmp_path: Path, mocker: MockerFixture, source: str, url: str +) -> None: + destination = tmp_path / "destination.tif" + error = subprocess.CalledProcessError(1, "gdal_translate") + mocker.patch("subprocess.check_call", side_effect=error) + is_not_found = mocker.patch("elevation.spatial.is_not_found", return_value=False) + + with pytest.raises(subprocess.CalledProcessError): + spatial.call_gdal_translate(source, destination, empty_on_notfound=True) + is_not_found.assert_called_once_with(url) + assert not destination.exists() + + is_not_found.return_value = True + with pytest.raises(subprocess.CalledProcessError): + spatial.call_gdal_translate(source, destination) + + spatial.call_gdal_translate(source, destination, empty_on_notfound=True) + assert destination.read_bytes() == b"" + + +def test_call_gdal_translate_empty_on_notfound_local( + tmp_path: Path, mocker: MockerFixture +) -> None: + is_not_found = mocker.patch("elevation.spatial.is_not_found") + + with pytest.raises(subprocess.CalledProcessError): + spatial.call_gdal_translate( + str(RASTER.with_suffix(".bad")), tmp_path / "x.tif", empty_on_notfound=True + ) + is_not_found.assert_not_called() + + def test_call_gdalbuildvrt_command(tmp_path: Path, mocker: MockerFixture) -> None: check_call = mocker.patch("subprocess.check_call") destination = tmp_path / "cache" / "destination.vrt" diff --git a/tests/test_20_datasource.py b/tests/test_20_datasource.py index 3043a67..417fdf0 100644 --- a/tests/test_20_datasource.py +++ b/tests/test_20_datasource.py @@ -170,6 +170,7 @@ def test_ensure_tiles(mocker: MockerFixture, tmp_path: Path) -> None: f"{spec['prepare_tile_kwargs']['gdal_source']}/N41/N41E012.hgt.gz", tmp_path / "spool" / "N41" / "N41E012.tif", options=spatial.INT_TILE_GDAL_OPTIONS, + empty_on_notfound=True, ) assert (tmp_path / "cache" / "N41" / "N41E012.tif").read_bytes() == b"tile" @@ -192,6 +193,7 @@ def test_ensure_tiles_vsi(mocker: MockerFixture, tmp_path: Path) -> None: f"{spec['prepare_tile_kwargs']['gdal_source']}/N41E012.tif", tmp_path / "spool" / "N41E012.tif", options=spatial.INT_TILE_GDAL_OPTIONS, + empty_on_notfound=True, ) assert (tmp_path / "cache" / "N41E012.tif").read_bytes() == b"tile" assert not (tmp_path / "spool" / "N41E012.tif").exists() @@ -231,6 +233,7 @@ def test_ensure_tiles_remote(mocker: MockerFixture, tmp_path: Path) -> None: tiles, prepare_tile=spec["prepare_tile"], gdal_options=spec["tile_gdal_options"], + empty_on_notfound=spec["empty_on_notfound"], **spec["prepare_tile_kwargs"], ) @@ -242,6 +245,7 @@ def test_ensure_tiles_remote(mocker: MockerFixture, tmp_path: Path) -> None: '/copernicus-dem/GLO-90-v1.zarr":/dsm', tmp_path / "spool" / "24/96.tif", options=spatial.FLOAT_TILE_GDAL_OPTIONS, + empty_on_notfound=False, ) assert (tmp_path / "cache" / "24/96.tif").read_bytes() == b"tile" @@ -265,6 +269,7 @@ def test_seed(mocker: MockerFixture, tmp_path: Path) -> None: f"{spec['prepare_tile_kwargs']['gdal_source']}/N43/N43E013.hgt.gz", datasource_root / "spool" / "N43" / "N43E013.tif", options=spatial.INT_TILE_GDAL_OPTIONS, + empty_on_notfound=True, ) assert mock_check_call.call_args[0][0][0] == "gdalbuildvrt" @@ -318,6 +323,7 @@ def test_seed_remote(mocker: MockerFixture, tmp_path: Path) -> None: '/copernicus-dem/GLO-30-v1.zarr":/dsm', datasource_root / "spool" / "96/192.tif", options=spatial.FLOAT_TILE_GDAL_OPTIONS, + empty_on_notfound=False, ) assert (datasource_root / "cache" / "96/192.tif").read_bytes() == b"tile" assert mock_check_call.call_args[0][0][0] == "gdalbuildvrt"