From 76f9e12d8524161843300708177e15d48b459cd3 Mon Sep 17 00:00:00 2001 From: williamloosman Date: Fri, 12 Jul 2024 14:38:51 +0200 Subject: [PATCH 1/2] Bump GDAL to 3.9.1 Fixed test matirx Changed error handling for GDAL in reverse compatible manner Fixed unit tests accordingly --- .github/workflows/pytest.yml | 8 +++---- Dockerfile | 4 ++-- geopackage_validator/utils.py | 9 ++++++- geopackage_validator/validate.py | 41 ++++++++++++++++++-------------- pyproject.toml | 5 ++-- tests/test_utils.py | 34 +++++++++++++++++++------- tests/test_validate.py | 7 ++++++ 7 files changed, 72 insertions(+), 36 deletions(-) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 8fad0f5..e0ad198 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -9,9 +9,9 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-22.04, ubuntu-20.04] # no ubuntugis @ ubuntu-24.04 - python-version: ['3.11', '3.10', '3.9', '3.8', '3.7'] # , '3.6'] <- 3.6 needs setup.cfg - gdal-version: ['3.4', '3.6'] # TODO: gdal 3.6 is still unstable. + os: [ubuntu-24.04] #, ubuntu-22.04, ubuntu-20.04] # no ubuntugis @ ubuntu-24.04 + python-version: ['3.11'] #, '3.10', '3.9', '3.8', '3.7'] # , '3.6'] <- 3.6 needs setup.cfg + gdal-version: ['3.8'] #, '3.6.2', '3.4'] exclude: - os: ubuntu-24.04 python-version: '3.8' @@ -43,7 +43,7 @@ jobs: echo adding ubuntugis stable sudo apt-add-repository ppa:ubuntugis/ppa fi - sudo apt-get update + sudo apt-get update || true echo available python3-gdal versions: $(apt-cache madison python3-gdal | cut -f2 -d "|" | tr -d " ") echo available libgdal-dev versions: $(apt-cache madison libgdal-dev | cut -f2 -d "|" | tr -d " ") export APT_GDAL_VERSION=$(apt-cache madison python3-gdal | grep ${{ matrix.gdal-version }} | head -n1 | cut -f2 -d "|" | tr -d " ") diff --git a/Dockerfile b/Dockerfile index 559717c..a73a8a6 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,6 @@ -ARG GDAL_VERSION=3.6.2 +ARG GDAL_VERSION=3.9.1 -FROM osgeo/gdal:alpine-normal-${GDAL_VERSION} AS base +FROM ghcr.io/osgeo/gdal:alpine-normal-${GDAL_VERSION} AS base LABEL maintainer="Roel van den Berg " diff --git a/geopackage_validator/utils.py b/geopackage_validator/utils.py index e974c5a..b95ece6 100644 --- a/geopackage_validator/utils.py +++ b/geopackage_validator/utils.py @@ -6,6 +6,8 @@ from pathlib import Path import json +from time import sleep + import yaml from typing import Callable, Optional @@ -57,7 +59,12 @@ def silence_gdal(): gdal.PushErrorHandler(error_handler) driver = ogr.GetDriverByName("GPKG") - dataset = driver.Open(filename, 0) + + dataset = None + try: + dataset = driver.Open(filename, 0) + except Exception as e: + error_handler(gdal.CE_Fatal, 0, e.args[0]) if dataset is not None: dataset.silence_gdal = silence_gdal diff --git a/geopackage_validator/validate.py b/geopackage_validator/validate.py index 832dd5d..3efda3e 100644 --- a/geopackage_validator/validate.py +++ b/geopackage_validator/validate.py @@ -2,6 +2,7 @@ import logging import sys import traceback +from time import sleep from osgeo import gdal @@ -77,29 +78,32 @@ def validators_to_use( return [validator_dict[code] for code in codes] +class GdalErrorHandler(object): + def __init__(self): + self.gdal_error_traces = [] + self.gdal_warning_traces = [] + + def handler(self, err_level, err_no, err_msg): + trace = err_msg.replace("\n", " ") + if err_level == gdal.CE_Warning: + self.gdal_warning_traces.append(trace) + else: + self.gdal_error_traces.append(trace) + + def validate( gpkg_path, table_definitions_path=None, validations_path=None, validations="" ): """Starts the geopackage validations.""" utils.check_gdal_version() - gdal_error_traces = [] - gdal_warning_traces = [] - - # Register GDAL error handler function - def gdal_error_handler(err_class, err_num, error): - trace = error.replace("\n", " ") - # import pdb - # pdb.set_trace() - if err_class == gdal.CE_Warning: - gdal_warning_traces.append(trace) - else: - gdal_error_traces.append(trace) + errHandler = GdalErrorHandler() + dataset = utils.open_dataset(gpkg_path, errHandler.handler) - dataset = utils.open_dataset(gpkg_path, gdal_error_handler) - if len(gdal_error_traces): + if len(errHandler.gdal_error_traces): initial_gdal_traces = [ - gdal_error_traces.pop() for _ in range(len(gdal_error_traces)) + errHandler.gdal_error_traces.pop() + for _ in range(len(errHandler.gdal_error_traces)) ] initial_gdal_errors = [ format_result( @@ -163,7 +167,8 @@ def gdal_error_handler(err_class, err_num, error): validation_error = True success = False current_gdal_error_traces = [ - gdal_error_traces.pop() for _ in range(len(gdal_error_traces)) + errHandler.gdal_error_traces.pop() + for _ in range(len(errHandler.gdal_error_traces)) ] if current_gdal_error_traces: success = False @@ -178,12 +183,12 @@ def gdal_error_handler(err_class, err_num, error): ) validation_results.append(output) - if gdal_warning_traces: + if errHandler.gdal_warning_traces: output = format_result( validation_code="UNKNOWN_WARNINGS", validation_description="It is recommended that these unexpected (GDAL) warnings are looked into.", level=ValidationLevel.UNKNOWN_WARNING, - trace=gdal_warning_traces, + trace=errHandler.gdal_warning_traces, ) validation_results.append(output) diff --git a/pyproject.toml b/pyproject.toml index 56e5aba..7e34b0d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,17 +10,18 @@ license = {text = "MIT"} classifiers = [ "Programming Language :: Python :: 3", "Programming Language :: Python :: 3 :: Only", - "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", + "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.11", ] dynamic = ["version", "readme"] dependencies = [ "setuptools>=42.0,!=58.*,!=59.*,!=60.*,!=61.*", "Click >= 8.0", "click-log >=0.3", - "gdal >=3.0.4", + "gdal >=3.4", "minio", "pyyaml", ] diff --git a/tests/test_utils.py b/tests/test_utils.py index c3a936f..ad6b724 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -24,9 +24,15 @@ def gdal_error_handler(err_class, err_num, error): results.append("GDAL_ERROR") dataset = open_dataset("tests/data/test_gdal_error.gpkg", gdal_error_handler) - validations = dataset.ExecuteSQL('select rtreecheck("rtree_table_geom");') - dataset.ReleaseResultSet(validations) - assert results[0] == "GDAL_ERROR" + + # Since GDAL 3.7 the exceptions work more pythonic + try: + validations = dataset.ExecuteSQL('select rtreecheck("rtree_table_geom");') + dataset.ReleaseResultSet(validations) + except RuntimeError as e: + results.append("GDAL_TRY_ERROR") + + assert results[0] == "GDAL_ERROR" or results[0] == "GDAL_TRY_ERROR" assert len(results) == 1 @@ -39,7 +45,9 @@ def gdal_error_handler(err_class, err_num, error): dataset = open_dataset("tests/data/test_gdal_error.gpkg", gdal_error_handler) with dataset.silence_gdal(): - validations = dataset.ExecuteSQL('select rtreecheck("rtree_table_geom");') + validations = dataset.ExecuteSQL( + 'select rtreecheck("rtree_cbs_arbeidsmarktregio_2014_gegeneraliseerd_geom_parent");' + ) dataset.ReleaseResultSet(validations) assert len(results) == 0 @@ -64,9 +72,17 @@ def gdal_error_handler(err_class, err_num, error): results.append("GDAL_ERROR") dataset = open_dataset("tests/data/test_gdal_error.gpkg", gdal_error_handler) - do_something_with_error_gdal(dataset) - do_something_silenced_gdal(dataset) - do_something_with_error_gdal(dataset) - assert len(results) == 2 - assert results == ["GDAL_ERROR", "GDAL_ERROR"] + # Since GDAL 3.7 this test will not work because these errors are thrown through the try except clause + try: + do_something_with_error_gdal(dataset) + do_something_silenced_gdal(dataset) + do_something_with_error_gdal(dataset) + except RuntimeError as e: + results.append("GDAL_TRY_ERROR") + + if len(results) == 1: + assert results == ["GDAL_TRY_ERROR"] + else: + assert len(results) == 2 + assert results == ["GDAL_ERROR", "GDAL_ERROR"] diff --git a/tests/test_validate.py b/tests/test_validate.py index bdeff13..cde6516 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -111,9 +111,16 @@ def test_validate_all_validations_no_error(): def test_validate_all_validations_with_broken_gpkg_throws_gdal_error(): + + # try: results, validations_executed, success = validate( gpkg_path="tests/data/test_broken_geopackage.gpkg", validations="ALL" ) + # except Exception: + # print('verwachte fout') + # else: + # print('foute fout') + assert len(results) == 1 print(results) assert results[0]["locations"] == [ From 2cc04be0f1d0671c97c89dd1abdf679df1479e67 Mon Sep 17 00:00:00 2001 From: williamloosman Date: Mon, 15 Jul 2024 14:51:30 +0200 Subject: [PATCH 2/2] Bump GDAL to 3.9.1 Fixed test matirx Changed error handling for GDAL in reverse compatible manner Fixed unit tests accordingly --- .github/workflows/pytest.yml | 18 +++++++++++++----- geopackage_validator/utils.py | 5 +---- geopackage_validator/validate.py | 1 - tests/test_utils.py | 25 +++++++++---------------- tests/test_validate.py | 5 ----- 5 files changed, 23 insertions(+), 31 deletions(-) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index e0ad198..d60fcfc 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -9,22 +9,30 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-24.04] #, ubuntu-22.04, ubuntu-20.04] # no ubuntugis @ ubuntu-24.04 - python-version: ['3.11'] #, '3.10', '3.9', '3.8', '3.7'] # , '3.6'] <- 3.6 needs setup.cfg - gdal-version: ['3.8'] #, '3.6.2', '3.4'] + os: [ubuntu-24.04, ubuntu-22.04, ubuntu-20.04] # no ubuntugis @ ubuntu-24.04 + python-version: ['3.11', '3.10', '3.9', '3.8', '3.7'] # , '3.6'] <- 3.6 needs setup.cfg + gdal-version: ['3.8', '3.6', '3.4'] exclude: - os: ubuntu-24.04 - python-version: '3.8' + python-version: '3.9' - os: ubuntu-24.04 python-version: '3.7' + - os: ubuntu-24.04 + python-version: '3.7' + - os: ubuntu-24.04 + gdal-version: '3.6' - os: ubuntu-24.04 gdal-version: '3.4' - os: ubuntu-22.04 gdal-version: '3.4' + - os: ubuntu-22.04 + gdal-version: '3.8' - os: ubuntu-20.04 python-version: '3.11' - os: ubuntu-20.04 gdal-version: '3.6' + - os: ubuntu-20.04 + gdal-version: '3.8' steps: - name: Checkout 🛎️ uses: actions/checkout@v3 @@ -43,7 +51,7 @@ jobs: echo adding ubuntugis stable sudo apt-add-repository ppa:ubuntugis/ppa fi - sudo apt-get update || true + sudo apt-get update || true # ignore erros becouse we can not install gdal from `ppa:ubuntugis/ppa` without apt errors echo available python3-gdal versions: $(apt-cache madison python3-gdal | cut -f2 -d "|" | tr -d " ") echo available libgdal-dev versions: $(apt-cache madison libgdal-dev | cut -f2 -d "|" | tr -d " ") export APT_GDAL_VERSION=$(apt-cache madison python3-gdal | grep ${{ matrix.gdal-version }} | head -n1 | cut -f2 -d "|" | tr -d " ") diff --git a/geopackage_validator/utils.py b/geopackage_validator/utils.py index b95ece6..403b392 100644 --- a/geopackage_validator/utils.py +++ b/geopackage_validator/utils.py @@ -6,12 +6,9 @@ from pathlib import Path import json -from time import sleep import yaml -from typing import Callable, Optional - try: from osgeo import ogr, osr, gdal @@ -64,7 +61,7 @@ def silence_gdal(): try: dataset = driver.Open(filename, 0) except Exception as e: - error_handler(gdal.CE_Fatal, 0, e.args[0]) + error_handler(gdal.CE_Failure, 0, e.args[0]) if dataset is not None: dataset.silence_gdal = silence_gdal diff --git a/geopackage_validator/validate.py b/geopackage_validator/validate.py index 3efda3e..0ee814a 100644 --- a/geopackage_validator/validate.py +++ b/geopackage_validator/validate.py @@ -2,7 +2,6 @@ import logging import sys import traceback -from time import sleep from osgeo import gdal diff --git a/tests/test_utils.py b/tests/test_utils.py index ad6b724..71b027d 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,3 +1,5 @@ +from osgeo import gdal + from geopackage_validator.utils import ( open_dataset, dataset_geometry_tables, @@ -53,14 +55,12 @@ def gdal_error_handler(err_class, err_num, error): def do_something_with_error_gdal(dataset): - validations = dataset.ExecuteSQL('select rtreecheck("rtree_table_geom");') - dataset.ReleaseResultSet(validations) + gdal.Error(gdal.CE_Warning, 9999, "test warning message") def do_something_silenced_gdal(dataset): with dataset.silence_gdal(): - validations = dataset.ExecuteSQL('select rtreecheck("rtree_table_geom");') - dataset.ReleaseResultSet(validations) + gdal.Error(gdal.CE_Warning, 9999, "test warning message") def test_silence_between_gdal_errors(): @@ -73,16 +73,9 @@ def gdal_error_handler(err_class, err_num, error): dataset = open_dataset("tests/data/test_gdal_error.gpkg", gdal_error_handler) - # Since GDAL 3.7 this test will not work because these errors are thrown through the try except clause - try: - do_something_with_error_gdal(dataset) - do_something_silenced_gdal(dataset) - do_something_with_error_gdal(dataset) - except RuntimeError as e: - results.append("GDAL_TRY_ERROR") + do_something_with_error_gdal(dataset) + do_something_silenced_gdal(dataset) + do_something_with_error_gdal(dataset) - if len(results) == 1: - assert results == ["GDAL_TRY_ERROR"] - else: - assert len(results) == 2 - assert results == ["GDAL_ERROR", "GDAL_ERROR"] + assert len(results) == 2 + assert results == ["GDAL_ERROR", "GDAL_ERROR"] diff --git a/tests/test_validate.py b/tests/test_validate.py index cde6516..8d37b8a 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -112,14 +112,9 @@ def test_validate_all_validations_no_error(): def test_validate_all_validations_with_broken_gpkg_throws_gdal_error(): - # try: results, validations_executed, success = validate( gpkg_path="tests/data/test_broken_geopackage.gpkg", validations="ALL" ) - # except Exception: - # print('verwachte fout') - # else: - # print('foute fout') assert len(results) == 1 print(results)