From 5de68fb4ca7abc941f1f2f6627edd3c840dba17e Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Thu, 30 Nov 2023 16:43:57 +0100 Subject: [PATCH 01/28] Add integration tests with responses for the EDR API. --- .gitignore | 1 + docker-compose.yml | 2 + integration-test/Dockerfile | 3 + integration-test/requirements.in | 2 + integration-test/requirements.txt | 30 +- .../capabilities/200/all_collections.json | 77 ++++ ...thin_an_area_with_multiple_parameters.json | 380 ++++++++++++++++++ .../200/locations_within_a_bbox.json | 27 ++ ...gle_location_with_multiple_parameters.json | 167 ++++++++ .../locations/404/no_data_found.json | 3 + ...e_coordinate_with_multiple_parameters.json | 167 ++++++++ .../metadata/200/single_collection.json | 67 +++ .../response/metadata/404/not_found.json | 3 + integration-test/test_api.py | 121 ++++++ 14 files changed, 1044 insertions(+), 6 deletions(-) create mode 100644 integration-test/response/capabilities/200/all_collections.json create mode 100644 integration-test/response/collection/area/200/data_within_an_area_with_multiple_parameters.json create mode 100644 integration-test/response/collection/locations/200/locations_within_a_bbox.json create mode 100644 integration-test/response/collection/locations/200/single_location_with_multiple_parameters.json create mode 100644 integration-test/response/collection/locations/404/no_data_found.json create mode 100644 integration-test/response/collection/position/200/single_coordinate_with_multiple_parameters.json create mode 100644 integration-test/response/metadata/200/single_collection.json create mode 100644 integration-test/response/metadata/404/not_found.json create mode 100644 integration-test/test_api.py diff --git a/.gitignore b/.gitignore index 1376338..63ba3bd 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ .run/ # Python +.pytest_cache/ __pycache__/ venv/ diff --git a/docker-compose.yml b/docker-compose.yml index 7d143e6..89ee4cf 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -49,6 +49,7 @@ services: condition: service_healthy api: + hostname: api build: context: . # TODO: Needed to get proto file. Find a better solution dockerfile: api/Dockerfile @@ -93,6 +94,7 @@ services: environment: - DSHOST=store - DSPORT=50050 + - BASE_URL=http://api:8000 depends_on: store: condition: service_healthy diff --git a/integration-test/Dockerfile b/integration-test/Dockerfile index 39a424a..eab301c 100644 --- a/integration-test/Dockerfile +++ b/integration-test/Dockerfile @@ -29,6 +29,9 @@ RUN python -m grpc_tools.protoc \ COPY "${PROJECT_PYTHON_PATH}/test_knmi.py" "${DOCKER_PATH}/test_knmi.py" COPY "${PROJECT_PYTHON_PATH}/test_delete.py" "${DOCKER_PATH}/test_delete.py" +COPY "${PROJECT_PYTHON_PATH}/test_api.py" "${DOCKER_PATH}/test_api.py" + +COPY "${PROJECT_PYTHON_PATH}/response/" "${DOCKER_PATH}/response/" WORKDIR "${DOCKER_PATH}" CMD ["pytest"] diff --git a/integration-test/requirements.in b/integration-test/requirements.in index d591866..3060a1e 100644 --- a/integration-test/requirements.in +++ b/integration-test/requirements.in @@ -3,5 +3,7 @@ # Install using: # pip-sync +deepdiff~=6.2 grpcio-tools~=1.56 pytest~=7.4 +requests~=2.31 diff --git a/integration-test/requirements.txt b/integration-test/requirements.txt index 1cb31cb..20bb3b0 100644 --- a/integration-test/requirements.txt +++ b/integration-test/requirements.txt @@ -1,23 +1,41 @@ # -# This file is autogenerated by pip-compile with Python 3.11 +# This file is autogenerated by pip-compile with Python 3.10 # by the following command: # # pip-compile --no-emit-index-url # -grpcio==1.58.0 +certifi==2023.11.17 + # via requests +charset-normalizer==3.3.2 + # via requests +deepdiff==6.7.1 + # via -r requirements.in +exceptiongroup==1.2.0 + # via pytest +grpcio==1.59.3 # via grpcio-tools -grpcio-tools==1.58.0 +grpcio-tools==1.59.3 # via -r requirements.in +idna==3.4 + # via requests iniconfig==2.0.0 # via pytest -packaging==23.1 +ordered-set==4.1.0 + # via deepdiff +packaging==23.2 # via pytest pluggy==1.3.0 # via pytest -protobuf==4.24.3 +protobuf==4.25.1 # via grpcio-tools -pytest==7.4.2 +pytest==7.4.3 # via -r requirements.in +requests==2.31.0 + # via -r requirements.in +tomli==2.0.1 + # via pytest +urllib3==2.1.0 + # via requests # The following packages are considered to be unsafe in a requirements file: # setuptools diff --git a/integration-test/response/capabilities/200/all_collections.json b/integration-test/response/capabilities/200/all_collections.json new file mode 100644 index 0000000..30ff65d --- /dev/null +++ b/integration-test/response/capabilities/200/all_collections.json @@ -0,0 +1,77 @@ +{ + "links": [ + { + "href": "http://localhost:8008/collections", + "rel": "self" + } + ], + "collections": [ + { + "id": "observations", + "links": [ + { + "href": "http://localhost:8008/collections/observations", + "rel": "self" + } + ], + "extent": { + "spatial": { + "bbox": [ + [ + 3.0, + 50.0, + 8.0, + 55.0 + ] + ], + "crs": "WGS84" + } + }, + "data_queries": { + "position": { + "link": { + "href": "http://localhost:8008/collections/observations/position", + "rel": "data", + "variables": { + "query_type": "position", + "output_format": [ + "CoverageJSON" + ] + } + } + }, + "area": { + "link": { + "href": "http://localhost:8008/collections/observations/area", + "rel": "data", + "variables": { + "query_type": "area", + "output_format": [ + "CoverageJSON" + ] + } + } + }, + "locations": { + "link": { + "href": "http://localhost:8008/collections/observations/locations", + "rel": "data", + "variables": { + "query_type": "locations", + "output_format": [ + "CoverageJSON" + ] + } + } + } + }, + "crs": [ + "WGS84" + ], + "output_formats": [ + "CoverageJSON" + ], + "parameter_names": {} + } + ] +} diff --git a/integration-test/response/collection/area/200/data_within_an_area_with_multiple_parameters.json b/integration-test/response/collection/area/200/data_within_an_area_with_multiple_parameters.json new file mode 100644 index 0000000..3f1d3b8 --- /dev/null +++ b/integration-test/response/collection/area/200/data_within_an_area_with_multiple_parameters.json @@ -0,0 +1,380 @@ +{ + "type": "CoverageCollection", + "coverages": [ + { + "type": "Coverage", + "domain": { + "type": "Domain", + "domainType": "PointSeries", + "axes": { + "x": { + "values": [ + 5.8723225499118 + ] + }, + "y": { + "values": [ + 52.0548617826 + ] + }, + "t": { + "values": [ + "2022-12-31T00:50:00Z", + "2022-12-31T01:00:00Z", + "2022-12-31T01:10:00Z", + "2022-12-31T01:20:00Z", + "2022-12-31T01:30:00Z", + "2022-12-31T01:40:00Z", + "2022-12-31T01:50:00Z", + "2022-12-31T02:00:00Z" + ] + } + }, + "referencing": [ + { + "coordinates": [ + "y", + "x" + ], + "system": { + "type": "GeographicCRS", + "id": "http://www.opengis.net/def/crs/EPSG/0/4326" + } + }, + { + "coordinates": [ + "z" + ], + "system": { + "type": "TemporalRS", + "calendar": "Gregorian" + } + } + ] + }, + "parameters": { + "dd": { + "type": "Parameter", + "observedProperty": { + "label": { + "en": "dd" + } + }, + "unit": { + "label": { + "en": "degree" + } + } + }, + "ff": { + "type": "Parameter", + "observedProperty": { + "label": { + "en": "ff" + } + }, + "unit": { + "label": { + "en": "m s-1" + } + } + }, + "rh": { + "type": "Parameter", + "observedProperty": { + "label": { + "en": "rh" + } + }, + "unit": { + "label": { + "en": "%" + } + } + } + }, + "ranges": { + "dd": { + "type": "NdArray", + "dataType": "float", + "axisNames": [ + "t", + "y", + "x" + ], + "shape": [ + 8, + 1, + 1 + ], + "values": [ + 236.4, + 240.3, + 250.4, + 239.2, + 232.7, + 241.4, + 247.9, + 248.1 + ] + }, + "ff": { + "type": "NdArray", + "dataType": "float", + "axisNames": [ + "t", + "y", + "x" + ], + "shape": [ + 8, + 1, + 1 + ], + "values": [ + 5.38, + 5.25, + 4.04, + 4.06, + 4.41, + 4.31, + 4.57, + 4.37 + ] + }, + "rh": { + "type": "NdArray", + "dataType": "float", + "axisNames": [ + "t", + "y", + "x" + ], + "shape": [ + 8, + 1, + 1 + ], + "values": [ + 97, + 96, + 96, + 96, + 96, + 97, + 96, + 95 + ] + } + } + }, + { + "type": "Coverage", + "domain": { + "type": "Domain", + "domainType": "PointSeries", + "axes": { + "x": { + "values": [ + 5.1797058644882 + ] + }, + "y": { + "values": [ + 52.098821802977 + ] + }, + "t": { + "values": [ + "2022-12-31T00:50:00Z", + "2022-12-31T01:00:00Z", + "2022-12-31T01:10:00Z", + "2022-12-31T01:20:00Z", + "2022-12-31T01:30:00Z", + "2022-12-31T01:40:00Z", + "2022-12-31T01:50:00Z", + "2022-12-31T02:00:00Z" + ] + } + }, + "referencing": [ + { + "coordinates": [ + "y", + "x" + ], + "system": { + "type": "GeographicCRS", + "id": "http://www.opengis.net/def/crs/EPSG/0/4326" + } + }, + { + "coordinates": [ + "z" + ], + "system": { + "type": "TemporalRS", + "calendar": "Gregorian" + } + } + ] + }, + "parameters": { + "dd": { + "type": "Parameter", + "observedProperty": { + "label": { + "en": "dd" + } + }, + "unit": { + "label": { + "en": "degree" + } + } + }, + "ff": { + "type": "Parameter", + "observedProperty": { + "label": { + "en": "ff" + } + }, + "unit": { + "label": { + "en": "m s-1" + } + } + }, + "rh": { + "type": "Parameter", + "observedProperty": { + "label": { + "en": "rh" + } + }, + "unit": { + "label": { + "en": "%" + } + } + } + }, + "ranges": { + "dd": { + "type": "NdArray", + "dataType": "float", + "axisNames": [ + "t", + "y", + "x" + ], + "shape": [ + 8, + 1, + 1 + ], + "values": [ + 237.9, + 235.4, + 240, + 243.2, + 243.2, + 240, + 239.9, + 229.5 + ] + }, + "ff": { + "type": "NdArray", + "dataType": "float", + "axisNames": [ + "t", + "y", + "x" + ], + "shape": [ + 8, + 1, + 1 + ], + "values": [ + 4.26, + 4.72, + 4.1, + 3.79, + 4.2, + 3.83, + 3.69, + 3.3 + ] + }, + "rh": { + "type": "NdArray", + "dataType": "float", + "axisNames": [ + "t", + "y", + "x" + ], + "shape": [ + 8, + 1, + 1 + ], + "values": [ + 95, + 94, + 94, + 94, + 94, + 93, + 93, + 94 + ] + } + } + } + ], + "parameters": { + "dd": { + "type": "Parameter", + "observedProperty": { + "label": { + "en": "dd" + } + }, + "unit": { + "label": { + "en": "degree" + } + } + }, + "ff": { + "type": "Parameter", + "observedProperty": { + "label": { + "en": "ff" + } + }, + "unit": { + "label": { + "en": "m s-1" + } + } + }, + "rh": { + "type": "Parameter", + "observedProperty": { + "label": { + "en": "rh" + } + }, + "unit": { + "label": { + "en": "%" + } + } + } + } +} diff --git a/integration-test/response/collection/locations/200/locations_within_a_bbox.json b/integration-test/response/collection/locations/200/locations_within_a_bbox.json new file mode 100644 index 0000000..5759e01 --- /dev/null +++ b/integration-test/response/collection/locations/200/locations_within_a_bbox.json @@ -0,0 +1,27 @@ +{ + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "geometry": { + "type": "Point", + "coordinates": [ + 5.1797058644882, + 52.098821802977 + ] + }, + "id": "06260" + }, + { + "type": "Feature", + "geometry": { + "type": "Point", + "coordinates": [ + 5.8723225499118, + 52.0548617826 + ] + }, + "id": "06275" + } + ] +} diff --git a/integration-test/response/collection/locations/200/single_location_with_multiple_parameters.json b/integration-test/response/collection/locations/200/single_location_with_multiple_parameters.json new file mode 100644 index 0000000..ee1da91 --- /dev/null +++ b/integration-test/response/collection/locations/200/single_location_with_multiple_parameters.json @@ -0,0 +1,167 @@ +{ + "type": "Coverage", + "domain": { + "type": "Domain", + "domainType": "PointSeries", + "axes": { + "x": { + "values": [ + 5.1797058644882 + ] + }, + "y": { + "values": [ + 52.098821802977 + ] + }, + "t": { + "values": [ + "2022-12-31T00:50:00Z", + "2022-12-31T01:00:00Z", + "2022-12-31T01:10:00Z", + "2022-12-31T01:20:00Z", + "2022-12-31T01:30:00Z", + "2022-12-31T01:40:00Z", + "2022-12-31T01:50:00Z", + "2022-12-31T02:00:00Z" + ] + } + }, + "referencing": [ + { + "coordinates": [ + "y", + "x" + ], + "system": { + "type": "GeographicCRS", + "id": "http://www.opengis.net/def/crs/EPSG/0/4326" + } + }, + { + "coordinates": [ + "z" + ], + "system": { + "type": "TemporalRS", + "calendar": "Gregorian" + } + } + ] + }, + "parameters": { + "dd": { + "type": "Parameter", + "observedProperty": { + "label": { + "en": "dd" + } + }, + "unit": { + "label": { + "en": "degree" + } + } + }, + "ff": { + "type": "Parameter", + "observedProperty": { + "label": { + "en": "ff" + } + }, + "unit": { + "label": { + "en": "m s-1" + } + } + }, + "rh": { + "type": "Parameter", + "observedProperty": { + "label": { + "en": "rh" + } + }, + "unit": { + "label": { + "en": "%" + } + } + } + }, + "ranges": { + "dd": { + "type": "NdArray", + "dataType": "float", + "axisNames": [ + "t", + "y", + "x" + ], + "shape": [ + 8, + 1, + 1 + ], + "values": [ + 237.9, + 235.4, + 240, + 243.2, + 243.2, + 240, + 239.9, + 229.5 + ] + }, + "ff": { + "type": "NdArray", + "dataType": "float", + "axisNames": [ + "t", + "y", + "x" + ], + "shape": [ + 8, + 1, + 1 + ], + "values": [ + 4.26, + 4.72, + 4.1, + 3.79, + 4.2, + 3.83, + 3.69, + 3.3 + ] + }, + "rh": { + "type": "NdArray", + "dataType": "float", + "axisNames": [ + "t", + "y", + "x" + ], + "shape": [ + 8, + 1, + 1 + ], + "values": [ + 95, + 94, + 94, + 94, + 94, + 93, + 93, + 94 + ] + } + } +} diff --git a/integration-test/response/collection/locations/404/no_data_found.json b/integration-test/response/collection/locations/404/no_data_found.json new file mode 100644 index 0000000..732842a --- /dev/null +++ b/integration-test/response/collection/locations/404/no_data_found.json @@ -0,0 +1,3 @@ +{ + "detail": "No data found" +} diff --git a/integration-test/response/collection/position/200/single_coordinate_with_multiple_parameters.json b/integration-test/response/collection/position/200/single_coordinate_with_multiple_parameters.json new file mode 100644 index 0000000..ee1da91 --- /dev/null +++ b/integration-test/response/collection/position/200/single_coordinate_with_multiple_parameters.json @@ -0,0 +1,167 @@ +{ + "type": "Coverage", + "domain": { + "type": "Domain", + "domainType": "PointSeries", + "axes": { + "x": { + "values": [ + 5.1797058644882 + ] + }, + "y": { + "values": [ + 52.098821802977 + ] + }, + "t": { + "values": [ + "2022-12-31T00:50:00Z", + "2022-12-31T01:00:00Z", + "2022-12-31T01:10:00Z", + "2022-12-31T01:20:00Z", + "2022-12-31T01:30:00Z", + "2022-12-31T01:40:00Z", + "2022-12-31T01:50:00Z", + "2022-12-31T02:00:00Z" + ] + } + }, + "referencing": [ + { + "coordinates": [ + "y", + "x" + ], + "system": { + "type": "GeographicCRS", + "id": "http://www.opengis.net/def/crs/EPSG/0/4326" + } + }, + { + "coordinates": [ + "z" + ], + "system": { + "type": "TemporalRS", + "calendar": "Gregorian" + } + } + ] + }, + "parameters": { + "dd": { + "type": "Parameter", + "observedProperty": { + "label": { + "en": "dd" + } + }, + "unit": { + "label": { + "en": "degree" + } + } + }, + "ff": { + "type": "Parameter", + "observedProperty": { + "label": { + "en": "ff" + } + }, + "unit": { + "label": { + "en": "m s-1" + } + } + }, + "rh": { + "type": "Parameter", + "observedProperty": { + "label": { + "en": "rh" + } + }, + "unit": { + "label": { + "en": "%" + } + } + } + }, + "ranges": { + "dd": { + "type": "NdArray", + "dataType": "float", + "axisNames": [ + "t", + "y", + "x" + ], + "shape": [ + 8, + 1, + 1 + ], + "values": [ + 237.9, + 235.4, + 240, + 243.2, + 243.2, + 240, + 239.9, + 229.5 + ] + }, + "ff": { + "type": "NdArray", + "dataType": "float", + "axisNames": [ + "t", + "y", + "x" + ], + "shape": [ + 8, + 1, + 1 + ], + "values": [ + 4.26, + 4.72, + 4.1, + 3.79, + 4.2, + 3.83, + 3.69, + 3.3 + ] + }, + "rh": { + "type": "NdArray", + "dataType": "float", + "axisNames": [ + "t", + "y", + "x" + ], + "shape": [ + 8, + 1, + 1 + ], + "values": [ + 95, + 94, + 94, + 94, + 94, + 93, + 93, + 94 + ] + } + } +} diff --git a/integration-test/response/metadata/200/single_collection.json b/integration-test/response/metadata/200/single_collection.json new file mode 100644 index 0000000..0043e4a --- /dev/null +++ b/integration-test/response/metadata/200/single_collection.json @@ -0,0 +1,67 @@ +{ + "id": "observations", + "links": [ + { + "href": "http://localhost:8008/collections/observations/observations", + "rel": "self" + } + ], + "extent": { + "spatial": { + "bbox": [ + [ + 3, + 50, + 8, + 55 + ] + ], + "crs": "WGS84" + } + }, + "data_queries": { + "position": { + "link": { + "href": "http://localhost:8008/collections/observations/observations/position", + "rel": "data", + "variables": { + "query_type": "position", + "output_format": [ + "CoverageJSON" + ] + } + } + }, + "area": { + "link": { + "href": "http://localhost:8008/collections/observations/observations/area", + "rel": "data", + "variables": { + "query_type": "area", + "output_format": [ + "CoverageJSON" + ] + } + } + }, + "locations": { + "link": { + "href": "http://localhost:8008/collections/observations/observations/locations", + "rel": "data", + "variables": { + "query_type": "locations", + "output_format": [ + "CoverageJSON" + ] + } + } + } + }, + "crs": [ + "WGS84" + ], + "output_formats": [ + "CoverageJSON" + ], + "parameter_names": {} +} diff --git a/integration-test/response/metadata/404/not_found.json b/integration-test/response/metadata/404/not_found.json new file mode 100644 index 0000000..634080e --- /dev/null +++ b/integration-test/response/metadata/404/not_found.json @@ -0,0 +1,3 @@ +{ + "detail": "Not Found" +} diff --git a/integration-test/test_api.py b/integration-test/test_api.py new file mode 100644 index 0000000..759fa75 --- /dev/null +++ b/integration-test/test_api.py @@ -0,0 +1,121 @@ +import json +import logging +import os +from pathlib import Path + +import requests +from deepdiff import DeepDiff + +logging.basicConfig() +logger = logging.getLogger(__name__) +logger.setLevel(os.environ.get("LOG_LEVEL", logging.INFO)) + + +BASE_URL = os.environ.get("BASE_URL", "http://localhost:8008") + + +def actual_status_code_is_expected_status_code(actual_response, expected_status_code): + assert actual_response.status_code == expected_status_code + + +def actual_response_is_expected_response(actual_response, expected_path): + file_path = Path(Path(__file__).parent, expected_path).resolve() + with open(file_path) as file: + expected_json = json.load(file) + + diff = DeepDiff(expected_json, actual_response.json(), ignore_order=True) + assert diff == {} + + +def test_get_all_collections(): + actual_response = requests.get(url=BASE_URL + "/collections") + + actual_status_code_is_expected_status_code(actual_response, 200) + actual_response_is_expected_response(actual_response, "response/capabilities/200/all_collections.json") + + +def test_get_a_single_existing_collection(): + collection_id = "observations" + actual_response = requests.get(url=BASE_URL + f"/collections/{collection_id}") + + actual_status_code_is_expected_status_code(actual_response, 200) + actual_response_is_expected_response(actual_response, "response/metadata/200/single_collection.json") + + +def test_get_a_collection_which_does_not_exist(): + collection_id = "does-not-exist" + actual_response = requests.get(url=BASE_URL + f"/collections/{collection_id}") + + actual_status_code_is_expected_status_code(actual_response, 404) + actual_response_is_expected_response(actual_response, "response/metadata/404/not_found.json") + + +def test_from_a_single_collection_get_locations_within_a_bbox(): + collection_id = "observations" + bbox = "5.0,52.0,6.0,52.1" + actual_response = requests.get(url=BASE_URL + f"/collections/{collection_id}/locations?bbox={bbox}") + + actual_status_code_is_expected_status_code(actual_response, 200) + actual_response_is_expected_response( + actual_response, "response/collection/locations/200/locations_within_a_bbox.json" + ) + + +def test_from_a_single_collection_get_a_single_location(): + collection_id = "observations" + location_id = "06260" + parameters = "dd,ff,rh" + datetime = "2022-12-31T00:50:00Z/2022-12-31T02:10:00Z" + actual_response = requests.get( + url=BASE_URL + f"/collections/{collection_id}/locations/{location_id}" + f"?parameter-name={parameters}&datetime={datetime}" + ) + + actual_status_code_is_expected_status_code(actual_response, 200) + actual_response_is_expected_response( + actual_response, "response/collection/locations/200/single_location_with_multiple_parameters.json" + ) + + +def test_from_a_single_collection_get_a_single_location_which_does_not_exist(): + collection_id = "observations" + location_id = "does-not-exist" + parameters = "does-not-exist" + actual_response = requests.get( + url=BASE_URL + f"/collections/{collection_id}/locations/{location_id}?parameter-name={parameters}" + ) + + actual_status_code_is_expected_status_code(actual_response, 404) + actual_response_is_expected_response(actual_response, "response/collection/locations/404/no_data_found.json") + + +def test_from_a_single_collection_get_a_single_position_with_multiple_parameters(): + collection_id = "observations" + coords = "POINT(5.179705 52.0988218)" + parameters = "dd,ff,rh" + datetime = "2022-12-31T00:50:00Z/2022-12-31T02:10:00Z" + actual_response = requests.get( + url=BASE_URL + f"/collections/{collection_id}/position" + f"?coords={coords}¶meter-name={parameters}&datetime={datetime}" + ) + + actual_status_code_is_expected_status_code(actual_response, 200) + actual_response_is_expected_response( + actual_response, "response/collection/position/200/single_coordinate_with_multiple_parameters.json" + ) + + +def test_from_a_single_collection_get_an_area_with_multiple_parameters(): + collection_id = "observations" + coords = "POLYGON((5.0 52.0, 6.0 52.0,6.0 52.1,5.0 52.1, 5.0 52.0))" + parameters = "dd,ff,rh" + datetime = "2022-12-31T00:50:00Z/2022-12-31T02:10:00Z" + actual_response = requests.get( + url=BASE_URL + f"/collections/{collection_id}/area" + f"?coords={coords}¶meter-name={parameters}&datetime={datetime}" + ) + + actual_status_code_is_expected_status_code(actual_response, 200) + actual_response_is_expected_response( + actual_response, "response/collection/area/200/data_within_an_area_with_multiple_parameters.json" + ) From 7c3938fe97f5e2e6e4ff0ee8d64d9b7174c10206 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Thu, 30 Nov 2023 18:02:22 +0100 Subject: [PATCH 02/28] Ignore href because the url differs based on running it locally or in the docker compose. --- integration-test/test_api.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/integration-test/test_api.py b/integration-test/test_api.py index 759fa75..a991327 100644 --- a/integration-test/test_api.py +++ b/integration-test/test_api.py @@ -18,12 +18,12 @@ def actual_status_code_is_expected_status_code(actual_response, expected_status_ assert actual_response.status_code == expected_status_code -def actual_response_is_expected_response(actual_response, expected_path): +def actual_response_is_expected_response(actual_response, expected_path, **kwargs): file_path = Path(Path(__file__).parent, expected_path).resolve() with open(file_path) as file: expected_json = json.load(file) - diff = DeepDiff(expected_json, actual_response.json(), ignore_order=True) + diff = DeepDiff(expected_json, actual_response.json(), ignore_order=True, **kwargs) assert diff == {} @@ -31,7 +31,9 @@ def test_get_all_collections(): actual_response = requests.get(url=BASE_URL + "/collections") actual_status_code_is_expected_status_code(actual_response, 200) - actual_response_is_expected_response(actual_response, "response/capabilities/200/all_collections.json") + actual_response_is_expected_response( + actual_response, "response/capabilities/200/all_collections.json", exclude_regex_paths=r".*\['href'\]$" + ) def test_get_a_single_existing_collection(): @@ -39,7 +41,9 @@ def test_get_a_single_existing_collection(): actual_response = requests.get(url=BASE_URL + f"/collections/{collection_id}") actual_status_code_is_expected_status_code(actual_response, 200) - actual_response_is_expected_response(actual_response, "response/metadata/200/single_collection.json") + actual_response_is_expected_response( + actual_response, "response/metadata/200/single_collection.json", exclude_regex_paths=r".*\['href'\]$" + ) def test_get_a_collection_which_does_not_exist(): From 2b5a8881a68860c136bde3db6b164b2a277cd92d Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Fri, 1 Dec 2023 14:35:03 +0100 Subject: [PATCH 03/28] Compile requirements with 3.11. --- integration-test/requirements.txt | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/integration-test/requirements.txt b/integration-test/requirements.txt index 20bb3b0..390448c 100644 --- a/integration-test/requirements.txt +++ b/integration-test/requirements.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.10 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # # pip-compile --no-emit-index-url @@ -10,13 +10,11 @@ charset-normalizer==3.3.2 # via requests deepdiff==6.7.1 # via -r requirements.in -exceptiongroup==1.2.0 - # via pytest grpcio==1.59.3 # via grpcio-tools grpcio-tools==1.59.3 # via -r requirements.in -idna==3.4 +idna==3.6 # via requests iniconfig==2.0.0 # via pytest @@ -32,8 +30,6 @@ pytest==7.4.3 # via -r requirements.in requests==2.31.0 # via -r requirements.in -tomli==2.0.1 - # via pytest urllib3==2.1.0 # via requests From f5b17ad8f7beb1196a1daac692e2c2af46b76906 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Fri, 1 Dec 2023 14:35:34 +0100 Subject: [PATCH 04/28] Update regex. --- integration-test/test_api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-test/test_api.py b/integration-test/test_api.py index a991327..eaa931e 100644 --- a/integration-test/test_api.py +++ b/integration-test/test_api.py @@ -32,7 +32,7 @@ def test_get_all_collections(): actual_status_code_is_expected_status_code(actual_response, 200) actual_response_is_expected_response( - actual_response, "response/capabilities/200/all_collections.json", exclude_regex_paths=r".*\['href'\]$" + actual_response, "response/capabilities/200/all_collections.json", exclude_regex_paths=r"\['href'\]$" ) @@ -42,7 +42,7 @@ def test_get_a_single_existing_collection(): actual_status_code_is_expected_status_code(actual_response, 200) actual_response_is_expected_response( - actual_response, "response/metadata/200/single_collection.json", exclude_regex_paths=r".*\['href'\]$" + actual_response, "response/metadata/200/single_collection.json", exclude_regex_paths=r"\['href'\]$" ) From 09975eadcd6e3da12e24505978099b68e659a513 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Fri, 1 Dec 2023 14:38:06 +0100 Subject: [PATCH 05/28] Remove ignore order from deepdiff. --- integration-test/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-test/test_api.py b/integration-test/test_api.py index eaa931e..40ccfae 100644 --- a/integration-test/test_api.py +++ b/integration-test/test_api.py @@ -23,7 +23,7 @@ def actual_response_is_expected_response(actual_response, expected_path, **kwarg with open(file_path) as file: expected_json = json.load(file) - diff = DeepDiff(expected_json, actual_response.json(), ignore_order=True, **kwargs) + diff = DeepDiff(expected_json, actual_response.json(), **kwargs) assert diff == {} From 2097ce1f978c1cda61b5f695e3bd744786d3f071 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Fri, 1 Dec 2023 15:12:19 +0100 Subject: [PATCH 06/28] Variance in datetime parameter. --- ...thin_an_area_with_multiple_parameters.json | 132 ++++++++---------- ...gle_location_with_multiple_parameters.json | 54 ++++--- integration-test/test_api.py | 4 +- 3 files changed, 89 insertions(+), 101 deletions(-) diff --git a/integration-test/response/collection/area/200/data_within_an_area_with_multiple_parameters.json b/integration-test/response/collection/area/200/data_within_an_area_with_multiple_parameters.json index 3f1d3b8..9d4e190 100644 --- a/integration-test/response/collection/area/200/data_within_an_area_with_multiple_parameters.json +++ b/integration-test/response/collection/area/200/data_within_an_area_with_multiple_parameters.json @@ -19,14 +19,13 @@ }, "t": { "values": [ - "2022-12-31T00:50:00Z", - "2022-12-31T01:00:00Z", - "2022-12-31T01:10:00Z", - "2022-12-31T01:20:00Z", - "2022-12-31T01:30:00Z", - "2022-12-31T01:40:00Z", - "2022-12-31T01:50:00Z", - "2022-12-31T02:00:00Z" + "2022-12-31T22:50:00Z", + "2022-12-31T23:00:00Z", + "2022-12-31T23:10:00Z", + "2022-12-31T23:20:00Z", + "2022-12-31T23:30:00Z", + "2022-12-31T23:40:00Z", + "2022-12-31T23:50:00Z" ] } }, @@ -103,19 +102,18 @@ "x" ], "shape": [ - 8, + 7, 1, 1 ], "values": [ - 236.4, - 240.3, - 250.4, - 239.2, - 232.7, - 241.4, - 247.9, - 248.1 + 214.8, + 215, + 212.5, + 213.9, + 216.6, + 213.9, + 212.3 ] }, "ff": { @@ -127,19 +125,18 @@ "x" ], "shape": [ - 8, + 7, 1, 1 ], "values": [ - 5.38, - 5.25, - 4.04, - 4.06, - 4.41, - 4.31, - 4.57, - 4.37 + 10.68, + 8.84, + 9.09, + 8.64, + 8.72, + 9.59, + 10.7 ] }, "rh": { @@ -151,19 +148,18 @@ "x" ], "shape": [ - 8, + 7, 1, 1 ], "values": [ - 97, - 96, - 96, - 96, - 96, - 97, - 96, - 95 + 56, + 56, + 55, + 58, + 56, + 56, + 57 ] } } @@ -186,14 +182,13 @@ }, "t": { "values": [ - "2022-12-31T00:50:00Z", - "2022-12-31T01:00:00Z", - "2022-12-31T01:10:00Z", - "2022-12-31T01:20:00Z", - "2022-12-31T01:30:00Z", - "2022-12-31T01:40:00Z", - "2022-12-31T01:50:00Z", - "2022-12-31T02:00:00Z" + "2022-12-31T22:50:00Z", + "2022-12-31T23:00:00Z", + "2022-12-31T23:10:00Z", + "2022-12-31T23:20:00Z", + "2022-12-31T23:30:00Z", + "2022-12-31T23:40:00Z", + "2022-12-31T23:50:00Z" ] } }, @@ -270,19 +265,18 @@ "x" ], "shape": [ - 8, + 7, 1, 1 ], "values": [ - 237.9, - 235.4, - 240, - 243.2, - 243.2, - 240, - 239.9, - 229.5 + 211.5, + 211.2, + 209.6, + 213.9, + 216.3, + 213.7, + 219.1 ] }, "ff": { @@ -294,19 +288,18 @@ "x" ], "shape": [ - 8, + 7, 1, 1 ], "values": [ - 4.26, - 4.72, - 4.1, - 3.79, - 4.2, - 3.83, - 3.69, - 3.3 + 8.37, + 7.71, + 8.35, + 8.45, + 8.95, + 9.17, + 9.4 ] }, "rh": { @@ -318,19 +311,18 @@ "x" ], "shape": [ - 8, + 7, 1, 1 ], "values": [ - 95, - 94, - 94, - 94, - 94, - 93, - 93, - 94 + 58, + 58, + 58, + 58, + 58, + 58, + 59 ] } } diff --git a/integration-test/response/collection/locations/200/single_location_with_multiple_parameters.json b/integration-test/response/collection/locations/200/single_location_with_multiple_parameters.json index ee1da91..e9dc6d8 100644 --- a/integration-test/response/collection/locations/200/single_location_with_multiple_parameters.json +++ b/integration-test/response/collection/locations/200/single_location_with_multiple_parameters.json @@ -16,14 +16,13 @@ }, "t": { "values": [ + "2022-12-31T00:00:00Z", + "2022-12-31T00:10:00Z", + "2022-12-31T00:20:00Z", + "2022-12-31T00:30:00Z", + "2022-12-31T00:40:00Z", "2022-12-31T00:50:00Z", - "2022-12-31T01:00:00Z", - "2022-12-31T01:10:00Z", - "2022-12-31T01:20:00Z", - "2022-12-31T01:30:00Z", - "2022-12-31T01:40:00Z", - "2022-12-31T01:50:00Z", - "2022-12-31T02:00:00Z" + "2022-12-31T01:00:00Z" ] } }, @@ -100,19 +99,18 @@ "x" ], "shape": [ - 8, + 7, 1, 1 ], "values": [ + 224.3, + 226, + 228.3, + 230.3, + 234.9, 237.9, - 235.4, - 240, - 243.2, - 243.2, - 240, - 239.9, - 229.5 + 235.4 ] }, "ff": { @@ -124,19 +122,18 @@ "x" ], "shape": [ - 8, + 7, 1, 1 ], "values": [ - 4.26, - 4.72, - 4.1, - 3.79, + 4.95, + 4.43, + 4.35, + 3.77, 4.2, - 3.83, - 3.69, - 3.3 + 4.26, + 4.72 ] }, "rh": { @@ -148,18 +145,17 @@ "x" ], "shape": [ - 8, + 7, 1, 1 ], "values": [ 95, 94, - 94, - 94, - 94, - 93, - 93, + 95, + 95, + 95, + 95, 94 ] } diff --git a/integration-test/test_api.py b/integration-test/test_api.py index 40ccfae..c3ce080 100644 --- a/integration-test/test_api.py +++ b/integration-test/test_api.py @@ -69,7 +69,7 @@ def test_from_a_single_collection_get_a_single_location(): collection_id = "observations" location_id = "06260" parameters = "dd,ff,rh" - datetime = "2022-12-31T00:50:00Z/2022-12-31T02:10:00Z" + datetime = "../2022-12-31T01:10:00Z" actual_response = requests.get( url=BASE_URL + f"/collections/{collection_id}/locations/{location_id}" f"?parameter-name={parameters}&datetime={datetime}" @@ -113,7 +113,7 @@ def test_from_a_single_collection_get_an_area_with_multiple_parameters(): collection_id = "observations" coords = "POLYGON((5.0 52.0, 6.0 52.0,6.0 52.1,5.0 52.1, 5.0 52.0))" parameters = "dd,ff,rh" - datetime = "2022-12-31T00:50:00Z/2022-12-31T02:10:00Z" + datetime = "2022-12-31T22:50:00Z/.." actual_response = requests.get( url=BASE_URL + f"/collections/{collection_id}/area" f"?coords={coords}¶meter-name={parameters}&datetime={datetime}" From c1c2e275a9175152cfc26cf8e4d8a13e8f0cf5b2 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Fri, 1 Dec 2023 15:16:40 +0100 Subject: [PATCH 07/28] Remove function. --- integration-test/test_api.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/integration-test/test_api.py b/integration-test/test_api.py index c3ce080..fc33d76 100644 --- a/integration-test/test_api.py +++ b/integration-test/test_api.py @@ -14,10 +14,6 @@ BASE_URL = os.environ.get("BASE_URL", "http://localhost:8008") -def actual_status_code_is_expected_status_code(actual_response, expected_status_code): - assert actual_response.status_code == expected_status_code - - def actual_response_is_expected_response(actual_response, expected_path, **kwargs): file_path = Path(Path(__file__).parent, expected_path).resolve() with open(file_path) as file: @@ -30,7 +26,7 @@ def actual_response_is_expected_response(actual_response, expected_path, **kwarg def test_get_all_collections(): actual_response = requests.get(url=BASE_URL + "/collections") - actual_status_code_is_expected_status_code(actual_response, 200) + assert actual_response.status_code == 200 actual_response_is_expected_response( actual_response, "response/capabilities/200/all_collections.json", exclude_regex_paths=r"\['href'\]$" ) @@ -40,7 +36,7 @@ def test_get_a_single_existing_collection(): collection_id = "observations" actual_response = requests.get(url=BASE_URL + f"/collections/{collection_id}") - actual_status_code_is_expected_status_code(actual_response, 200) + assert actual_response.status_code == 200 actual_response_is_expected_response( actual_response, "response/metadata/200/single_collection.json", exclude_regex_paths=r"\['href'\]$" ) @@ -50,7 +46,7 @@ def test_get_a_collection_which_does_not_exist(): collection_id = "does-not-exist" actual_response = requests.get(url=BASE_URL + f"/collections/{collection_id}") - actual_status_code_is_expected_status_code(actual_response, 404) + assert actual_response.status_code == 404 actual_response_is_expected_response(actual_response, "response/metadata/404/not_found.json") @@ -59,7 +55,7 @@ def test_from_a_single_collection_get_locations_within_a_bbox(): bbox = "5.0,52.0,6.0,52.1" actual_response = requests.get(url=BASE_URL + f"/collections/{collection_id}/locations?bbox={bbox}") - actual_status_code_is_expected_status_code(actual_response, 200) + assert actual_response.status_code == 200 actual_response_is_expected_response( actual_response, "response/collection/locations/200/locations_within_a_bbox.json" ) @@ -75,7 +71,7 @@ def test_from_a_single_collection_get_a_single_location(): f"?parameter-name={parameters}&datetime={datetime}" ) - actual_status_code_is_expected_status_code(actual_response, 200) + assert actual_response.status_code == 200 actual_response_is_expected_response( actual_response, "response/collection/locations/200/single_location_with_multiple_parameters.json" ) @@ -89,7 +85,7 @@ def test_from_a_single_collection_get_a_single_location_which_does_not_exist(): url=BASE_URL + f"/collections/{collection_id}/locations/{location_id}?parameter-name={parameters}" ) - actual_status_code_is_expected_status_code(actual_response, 404) + assert actual_response.status_code == 404 actual_response_is_expected_response(actual_response, "response/collection/locations/404/no_data_found.json") @@ -103,7 +99,7 @@ def test_from_a_single_collection_get_a_single_position_with_multiple_parameters f"?coords={coords}¶meter-name={parameters}&datetime={datetime}" ) - actual_status_code_is_expected_status_code(actual_response, 200) + assert actual_response.status_code == 200 actual_response_is_expected_response( actual_response, "response/collection/position/200/single_coordinate_with_multiple_parameters.json" ) @@ -119,7 +115,7 @@ def test_from_a_single_collection_get_an_area_with_multiple_parameters(): f"?coords={coords}¶meter-name={parameters}&datetime={datetime}" ) - actual_status_code_is_expected_status_code(actual_response, 200) + assert actual_response.status_code == 200 actual_response_is_expected_response( actual_response, "response/collection/area/200/data_within_an_area_with_multiple_parameters.json" ) From f18e9a8a9dde490dfa63b2018995d3d0f355545f Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Fri, 1 Dec 2023 15:30:39 +0100 Subject: [PATCH 08/28] Add one second to the end_datetime so that the end_datetime is included in the interval. --- api/main.py | 3 ++- ...ngle_location_with_multiple_parameters.json | 16 ++++++++++------ ...le_coordinate_with_multiple_parameters.json | 18 +++++++++++------- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/api/main.py b/api/main.py index de70c37..2313b9f 100644 --- a/api/main.py +++ b/api/main.py @@ -140,7 +140,8 @@ def get_datetime_range(datetime_string: str | None) -> Tuple[Timestamp, Timestam else: start_datetime.FromDatetime(datetime.min) if datetimes[1] != "..": - end_datetime.FromDatetime(aware_datetime_type_adapter.validate_python(datetimes[1])) + # HACK add one second so that the end_datetime is included in the interval. + end_datetime.FromDatetime(aware_datetime_type_adapter.validate_python(datetimes[1]) + timedelta(seconds=1)) else: end_datetime.FromDatetime(datetime.max) diff --git a/integration-test/response/collection/locations/200/single_location_with_multiple_parameters.json b/integration-test/response/collection/locations/200/single_location_with_multiple_parameters.json index e9dc6d8..1666c0a 100644 --- a/integration-test/response/collection/locations/200/single_location_with_multiple_parameters.json +++ b/integration-test/response/collection/locations/200/single_location_with_multiple_parameters.json @@ -22,7 +22,8 @@ "2022-12-31T00:30:00Z", "2022-12-31T00:40:00Z", "2022-12-31T00:50:00Z", - "2022-12-31T01:00:00Z" + "2022-12-31T01:00:00Z", + "2022-12-31T01:10:00Z" ] } }, @@ -99,7 +100,7 @@ "x" ], "shape": [ - 7, + 8, 1, 1 ], @@ -110,7 +111,8 @@ 230.3, 234.9, 237.9, - 235.4 + 235.4, + 240 ] }, "ff": { @@ -122,7 +124,7 @@ "x" ], "shape": [ - 7, + 8, 1, 1 ], @@ -133,7 +135,8 @@ 3.77, 4.2, 4.26, - 4.72 + 4.72, + 4.1 ] }, "rh": { @@ -145,7 +148,7 @@ "x" ], "shape": [ - 7, + 8, 1, 1 ], @@ -156,6 +159,7 @@ 95, 95, 95, + 94, 94 ] } diff --git a/integration-test/response/collection/position/200/single_coordinate_with_multiple_parameters.json b/integration-test/response/collection/position/200/single_coordinate_with_multiple_parameters.json index ee1da91..27a5ace 100644 --- a/integration-test/response/collection/position/200/single_coordinate_with_multiple_parameters.json +++ b/integration-test/response/collection/position/200/single_coordinate_with_multiple_parameters.json @@ -23,7 +23,8 @@ "2022-12-31T01:30:00Z", "2022-12-31T01:40:00Z", "2022-12-31T01:50:00Z", - "2022-12-31T02:00:00Z" + "2022-12-31T02:00:00Z", + "2022-12-31T02:10:00Z" ] } }, @@ -100,7 +101,7 @@ "x" ], "shape": [ - 8, + 9, 1, 1 ], @@ -112,7 +113,8 @@ 243.2, 240, 239.9, - 229.5 + 229.5, + 223.2 ] }, "ff": { @@ -124,7 +126,7 @@ "x" ], "shape": [ - 8, + 9, 1, 1 ], @@ -136,7 +138,8 @@ 4.2, 3.83, 3.69, - 3.3 + 3.3, + 4.04 ] }, "rh": { @@ -148,7 +151,7 @@ "x" ], "shape": [ - 8, + 9, 1, 1 ], @@ -160,7 +163,8 @@ 94, 93, 93, - 94 + 94, + 93 ] } } From 5f78d243109e90b71b160cd2a778e5fb1e373233 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Fri, 1 Dec 2023 15:42:48 +0100 Subject: [PATCH 09/28] Variance in the use of parameters. --- ...a_within_an_area_with_two_parameters.json} | 85 --------------- ...single_coordinate_with_one_parameter.json} | 102 +++--------------- integration-test/test_api.py | 12 +-- 3 files changed, 19 insertions(+), 180 deletions(-) rename integration-test/response/collection/area/200/{data_within_an_area_with_multiple_parameters.json => data_within_an_area_with_two_parameters.json} (77%) rename integration-test/response/collection/position/200/{single_coordinate_with_multiple_parameters.json => single_coordinate_with_one_parameter.json} (50%) diff --git a/integration-test/response/collection/area/200/data_within_an_area_with_multiple_parameters.json b/integration-test/response/collection/area/200/data_within_an_area_with_two_parameters.json similarity index 77% rename from integration-test/response/collection/area/200/data_within_an_area_with_multiple_parameters.json rename to integration-test/response/collection/area/200/data_within_an_area_with_two_parameters.json index 9d4e190..1878503 100644 --- a/integration-test/response/collection/area/200/data_within_an_area_with_multiple_parameters.json +++ b/integration-test/response/collection/area/200/data_within_an_area_with_two_parameters.json @@ -52,19 +52,6 @@ ] }, "parameters": { - "dd": { - "type": "Parameter", - "observedProperty": { - "label": { - "en": "dd" - } - }, - "unit": { - "label": { - "en": "degree" - } - } - }, "ff": { "type": "Parameter", "observedProperty": { @@ -93,29 +80,6 @@ } }, "ranges": { - "dd": { - "type": "NdArray", - "dataType": "float", - "axisNames": [ - "t", - "y", - "x" - ], - "shape": [ - 7, - 1, - 1 - ], - "values": [ - 214.8, - 215, - 212.5, - 213.9, - 216.6, - 213.9, - 212.3 - ] - }, "ff": { "type": "NdArray", "dataType": "float", @@ -215,19 +179,6 @@ ] }, "parameters": { - "dd": { - "type": "Parameter", - "observedProperty": { - "label": { - "en": "dd" - } - }, - "unit": { - "label": { - "en": "degree" - } - } - }, "ff": { "type": "Parameter", "observedProperty": { @@ -256,29 +207,6 @@ } }, "ranges": { - "dd": { - "type": "NdArray", - "dataType": "float", - "axisNames": [ - "t", - "y", - "x" - ], - "shape": [ - 7, - 1, - 1 - ], - "values": [ - 211.5, - 211.2, - 209.6, - 213.9, - 216.3, - 213.7, - 219.1 - ] - }, "ff": { "type": "NdArray", "dataType": "float", @@ -329,19 +257,6 @@ } ], "parameters": { - "dd": { - "type": "Parameter", - "observedProperty": { - "label": { - "en": "dd" - } - }, - "unit": { - "label": { - "en": "degree" - } - } - }, "ff": { "type": "Parameter", "observedProperty": { diff --git a/integration-test/response/collection/position/200/single_coordinate_with_multiple_parameters.json b/integration-test/response/collection/position/200/single_coordinate_with_one_parameter.json similarity index 50% rename from integration-test/response/collection/position/200/single_coordinate_with_multiple_parameters.json rename to integration-test/response/collection/position/200/single_coordinate_with_one_parameter.json index 27a5ace..f8b3846 100644 --- a/integration-test/response/collection/position/200/single_coordinate_with_multiple_parameters.json +++ b/integration-test/response/collection/position/200/single_coordinate_with_one_parameter.json @@ -51,98 +51,22 @@ ] }, "parameters": { - "dd": { + "tn": { "type": "Parameter", "observedProperty": { "label": { - "en": "dd" + "en": "tn" } }, "unit": { "label": { - "en": "degree" - } - } - }, - "ff": { - "type": "Parameter", - "observedProperty": { - "label": { - "en": "ff" - } - }, - "unit": { - "label": { - "en": "m s-1" - } - } - }, - "rh": { - "type": "Parameter", - "observedProperty": { - "label": { - "en": "rh" - } - }, - "unit": { - "label": { - "en": "%" + "en": "degrees Celsius" } } } }, "ranges": { - "dd": { - "type": "NdArray", - "dataType": "float", - "axisNames": [ - "t", - "y", - "x" - ], - "shape": [ - 9, - 1, - 1 - ], - "values": [ - 237.9, - 235.4, - 240, - 243.2, - 243.2, - 240, - 239.9, - 229.5, - 223.2 - ] - }, - "ff": { - "type": "NdArray", - "dataType": "float", - "axisNames": [ - "t", - "y", - "x" - ], - "shape": [ - 9, - 1, - 1 - ], - "values": [ - 4.26, - 4.72, - 4.1, - 3.79, - 4.2, - 3.83, - 3.69, - 3.3, - 4.04 - ] - }, - "rh": { + "tn": { "type": "NdArray", "dataType": "float", "axisNames": [ @@ -156,15 +80,15 @@ 1 ], "values": [ - 95, - 94, - 94, - 94, - 94, - 93, - 93, - 94, - 93 + 12.1, + 12, + 12, + 11.9, + 11.9, + 11.8, + 11.8, + 11.7, + 11.7 ] } } diff --git a/integration-test/test_api.py b/integration-test/test_api.py index fc33d76..f78067c 100644 --- a/integration-test/test_api.py +++ b/integration-test/test_api.py @@ -89,10 +89,10 @@ def test_from_a_single_collection_get_a_single_location_which_does_not_exist(): actual_response_is_expected_response(actual_response, "response/collection/locations/404/no_data_found.json") -def test_from_a_single_collection_get_a_single_position_with_multiple_parameters(): +def test_from_a_single_collection_get_a_single_position_with_one_parameter(): collection_id = "observations" coords = "POINT(5.179705 52.0988218)" - parameters = "dd,ff,rh" + parameters = "tn" datetime = "2022-12-31T00:50:00Z/2022-12-31T02:10:00Z" actual_response = requests.get( url=BASE_URL + f"/collections/{collection_id}/position" @@ -101,14 +101,14 @@ def test_from_a_single_collection_get_a_single_position_with_multiple_parameters assert actual_response.status_code == 200 actual_response_is_expected_response( - actual_response, "response/collection/position/200/single_coordinate_with_multiple_parameters.json" + actual_response, "response/collection/position/200/single_coordinate_with_one_parameter.json" ) -def test_from_a_single_collection_get_an_area_with_multiple_parameters(): +def test_from_a_single_collection_get_an_area_with_two_parameters(): collection_id = "observations" coords = "POLYGON((5.0 52.0, 6.0 52.0,6.0 52.1,5.0 52.1, 5.0 52.0))" - parameters = "dd,ff,rh" + parameters = " rh, ff " datetime = "2022-12-31T22:50:00Z/.." actual_response = requests.get( url=BASE_URL + f"/collections/{collection_id}/area" @@ -117,5 +117,5 @@ def test_from_a_single_collection_get_an_area_with_multiple_parameters(): assert actual_response.status_code == 200 actual_response_is_expected_response( - actual_response, "response/collection/area/200/data_within_an_area_with_multiple_parameters.json" + actual_response, "response/collection/area/200/data_within_an_area_with_two_parameters.json" ) From 63927ba98ce4288f31138a99e2474c15c6045e51 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Fri, 1 Dec 2023 15:53:48 +0100 Subject: [PATCH 10/28] Test the order of parameters. --- integration-test/test_api.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/integration-test/test_api.py b/integration-test/test_api.py index f78067c..157f307 100644 --- a/integration-test/test_api.py +++ b/integration-test/test_api.py @@ -76,6 +76,20 @@ def test_from_a_single_collection_get_a_single_location(): actual_response, "response/collection/locations/200/single_location_with_multiple_parameters.json" ) + # Test that we do not care about whitespace, by adding whitespace to the parameter string. + # Test that we do not care about the parameter order, by changing the order of parameter in the parameter string. + # Will return the same response as the first request. + parameters_2 = " rh, ff, dd " + actual_response_2 = requests.get( + url=BASE_URL + f"/collections/{collection_id}/locations/{location_id}" + f"?parameter-name={parameters_2}&datetime={datetime}" + ) + + assert actual_response_2.status_code == 200 + actual_response_is_expected_response( + actual_response_2, "response/collection/locations/200/single_location_with_multiple_parameters.json" + ) + def test_from_a_single_collection_get_a_single_location_which_does_not_exist(): collection_id = "observations" From 327aa1e90e1c0f8ed1805fcd8a64278182f36dd2 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Fri, 1 Dec 2023 17:39:12 +0100 Subject: [PATCH 11/28] Split the parameter order test from the other test. --- integration-test/test_api.py | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/integration-test/test_api.py b/integration-test/test_api.py index 157f307..4b02e68 100644 --- a/integration-test/test_api.py +++ b/integration-test/test_api.py @@ -76,20 +76,29 @@ def test_from_a_single_collection_get_a_single_location(): actual_response, "response/collection/locations/200/single_location_with_multiple_parameters.json" ) - # Test that we do not care about whitespace, by adding whitespace to the parameter string. - # Test that we do not care about the parameter order, by changing the order of parameter in the parameter string. - # Will return the same response as the first request. - parameters_2 = " rh, ff, dd " - actual_response_2 = requests.get( - url=BASE_URL + f"/collections/{collection_id}/locations/{location_id}" - f"?parameter-name={parameters_2}&datetime={datetime}" + +def test_that_the_order_of_the_parameters_in_the_response_is_always_the_same(): + """Test that we do not care about the order of parameters passed in the query. + By comparing two requests with the same parameters but in a different sequence. + The first request returns the same response as the second request. + """ + collection_id = "observations" + location_id = "06260" + parameters = " dd, ff , rh" + first_response = requests.get( + url=BASE_URL + f"/collections/{collection_id}/locations/{location_id}" f"?parameter-name={parameters}" ) - assert actual_response_2.status_code == 200 - actual_response_is_expected_response( - actual_response_2, "response/collection/locations/200/single_location_with_multiple_parameters.json" + parameters_2 = " rh, ff, dd " + second_response = requests.get( + url=BASE_URL + f"/collections/{collection_id}/locations/{location_id}" f"?parameter-name={parameters_2}" ) + assert first_response.status_code == 200 + assert second_response.status_code == 200 + diff = DeepDiff(first_response.json(), second_response.json()) + assert diff == {} + def test_from_a_single_collection_get_a_single_location_which_does_not_exist(): collection_id = "observations" From 24435b7ab6d4304a74aa4b094c1725452d205f75 Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Mon, 4 Dec 2023 13:40:45 +0100 Subject: [PATCH 12/28] Improved docs and simplified function --- .../postgresql/gettsattrgroups.go | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/datastore/storagebackend/postgresql/gettsattrgroups.go b/datastore/storagebackend/postgresql/gettsattrgroups.go index 09347f3..829284f 100644 --- a/datastore/storagebackend/postgresql/gettsattrgroups.go +++ b/datastore/storagebackend/postgresql/gettsattrgroups.go @@ -15,7 +15,7 @@ var ( tspb2go map[string]string // association between time series specific protobuf name and // (generated by protoc) Go name. // NOTES: - // - Protobuf names are field names the TSMetadata message in datastore.proto. + // - Protobuf names are field names of the TSMetadata message in datastore.proto. // - Protobuf names are identical to corresponding database column names. // - Protobuf names are snake case (aaa_bbb) whereas Go names are camel case (aaaBbb or AaaBbb). ) @@ -54,27 +54,25 @@ func getTSAllPBNames() []string { return pbNames } -// getTSDBColumns returns names of database columns corresponding to pbNames. -func getTSDBColumns(pbNames []string) ([]string, error) { +// validateAttrs validates pbNames. Returns nil if valid, otherwise error. +func validateAttrs(pbNames []string) error { seen := map[string]struct{}{} - cols := []string{} for _, pbName := range pbNames { if _, found := tspb2go[pbName]; !found { - return nil, fmt.Errorf( + return fmt.Errorf( "attribute not found: %s; supported attributes: %s", pbName, strings.Join(getTSAllPBNames(), ", ")) } if _, found := seen[pbName]; found { - return nil, fmt.Errorf("attribute %s specified more than once", pbName) + return fmt.Errorf("attribute %s specified more than once", pbName) } - cols = append(cols, pbName) seen[pbName] = struct{}{} } - return cols, nil + return nil } // getTSMdata returns a TSMetadata object initialized from colVals. @@ -305,19 +303,18 @@ func getTSAttrGroupsComboOnly(db *sql.DB, cols []string, groups *[]*datastore.TS func (sbe *PostgreSQL) GetTSAttrGroups(request *datastore.GetTSAGRequest) ( *datastore.GetTSAGResponse, error) { - cols, err := getTSDBColumns(request.Attrs) // get database column names for requested attributes - if err != nil { - return nil, fmt.Errorf("getTSAttrCols() failed: %v", err) + if err := validateAttrs(request.Attrs); err != nil { + return nil, fmt.Errorf("validateAttrs() failed: %v", err) } groups := []*datastore.TSMdataGroup{} if request.IncludeInstances { - if err := getTSAttrGroupsIncInstances(sbe.Db, cols, &groups); err != nil { + if err := getTSAttrGroupsIncInstances(sbe.Db, request.Attrs, &groups); err != nil { return nil, fmt.Errorf("getTSAGroupsIncInstances() failed: %v", err) } } else { - if err := getTSAttrGroupsComboOnly(sbe.Db, cols, &groups); err != nil { + if err := getTSAttrGroupsComboOnly(sbe.Db, request.Attrs, &groups); err != nil { return nil, fmt.Errorf("getTSAGroupsComboOnly() failed: %v", err) } } From 21ba2807aa995dba814a63007b5c8314ba34875a Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Mon, 4 Dec 2023 14:46:26 +0100 Subject: [PATCH 13/28] Improved function documentation --- datastore/storagebackend/postgresql/gettsattrgroups.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/datastore/storagebackend/postgresql/gettsattrgroups.go b/datastore/storagebackend/postgresql/gettsattrgroups.go index 829284f..293b2c2 100644 --- a/datastore/storagebackend/postgresql/gettsattrgroups.go +++ b/datastore/storagebackend/postgresql/gettsattrgroups.go @@ -117,24 +117,29 @@ func getTSMdata(colVals map[string]interface{}) (*datastore.TSMetadata, error) { return &tsMData, nil } -// scanTsMdata scans column cols from current result row in rows and converts to a -// TSMetadata object. +// scanTsMdata scans the current row in rows and converts the result into a TSMetadata object. +// It is assumed that cols contains exactly the columns (names, types, order) that were used for +// the query that resulted in rows. // Returns (TSMetadata object, nil) upon success, otherwise (..., error). func scanTsMdata(rows *sql.Rows, cols []string) (*datastore.TSMetadata, error) { + colVals0 := make([]interface{}, len(cols)) // column values colValPtrs := make([]interface{}, len(cols)) // pointers to column values for i := range colVals0 { colValPtrs[i] = &colVals0[i] } + // scan row into column value pointers if err := rows.Scan(colValPtrs...); err != nil { return nil, fmt.Errorf("rows.Scan() failed: %v", err) } + // combine column names and -values into a map colVals := map[string]interface{}{} for i, col := range cols { colVals[col] = colVals0[i] } + // convert to a TSMetadata object tsMdata, err := getTSMdata(colVals) if err != nil { From 2895add96497860b7ec66aa553d4959b31203dd5 Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Tue, 5 Dec 2023 09:54:44 +0100 Subject: [PATCH 14/28] Return new array instead updating existing one passed via pointer --- .../postgresql/gettsattrgroups.go | 59 ++++++++++--------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/datastore/storagebackend/postgresql/gettsattrgroups.go b/datastore/storagebackend/postgresql/gettsattrgroups.go index 293b2c2..0e5e47e 100644 --- a/datastore/storagebackend/postgresql/gettsattrgroups.go +++ b/datastore/storagebackend/postgresql/gettsattrgroups.go @@ -205,17 +205,17 @@ func getCombo(tsMdata1 *datastore.TSMetadata, goNames []string) (*datastore.TSMe return &tsMdata2, nil } -// getTSAttrGroupsIncInstances populates groups from cols such that each group contains all -// instances that match a unique combination of database values corresponding to cols. +// getTSAttrGroupsIncInstances creates an array of groups from cols such that each group contains +// all instances that match a unique combination of database values corresponding to cols. // All attributes, including those in cols, are set to the actual values found in the database. -// Returns nil upon success, otherwise error. +// Returns (array of groups, nil) upon success, otherwise (..., error). func getTSAttrGroupsIncInstances( - db *sql.DB, cols []string, groups *[]*datastore.TSMdataGroup) error { + db *sql.DB, cols []string) ([]*datastore.TSMdataGroup, error) { allCols := getTSAllPBNames() // get all protobuf names of TSMetadata message goNames, err := getTSGoNamesFromPBNames(cols) if err != nil { - return fmt.Errorf("getTSGoNamesFromPBNames() failed: %v", err) + return nil, fmt.Errorf("getTSGoNamesFromPBNames() failed: %v", err) } // query database for all columns in time_series, ordered by cols @@ -224,32 +224,34 @@ func getTSAttrGroupsIncInstances( query := fmt.Sprintf("SELECT %s FROM time_series ORDER BY %s", allColsS, colsS) rows, err := db.Query(query) if err != nil { - return fmt.Errorf("db.Query() failed: %v", err) + return nil, fmt.Errorf("db.Query() failed: %v", err) } defer rows.Close() + groups := []*datastore.TSMdataGroup{} + // aggregate rows into groups currInstances := []*datastore.TSMetadata{} // initial current instance set for rows.Next() { // extract tsMdata from current result row tsMdata, err := scanTsMdata(rows, allCols) if err != nil { - return fmt.Errorf("scanTsMdata() failed: %v", err) + return nil, fmt.Errorf("scanTsMdata() failed: %v", err) } if len(currInstances) > 0 { // check if we should create a new current instance set equal, err := tsMdataEqual(tsMdata, currInstances[0], goNames) if err != nil { - return fmt.Errorf("tsMdataEqual() failed: %v", err) + return nil, fmt.Errorf("tsMdataEqual() failed: %v", err) } if !equal { // ts metadata changed wrt. cols // add next group with current instance set currCombo, err := getCombo(currInstances[0], goNames) if err != nil { - return fmt.Errorf("getCombo() failed (1): %v", err) + return nil, fmt.Errorf("getCombo() failed (1): %v", err) } - *groups = append(*groups, &datastore.TSMdataGroup{ + groups = append(groups, &datastore.TSMdataGroup{ Combo: currCombo, Instances: currInstances, }) @@ -264,44 +266,46 @@ func getTSAttrGroupsIncInstances( // add final group with current instance set currCombo, err := getCombo(currInstances[0], goNames) if err != nil { - return fmt.Errorf("getCombo() failed (2): %v", err) + return nil, fmt.Errorf("getCombo() failed (2): %v", err) } - *groups = append(*groups, &datastore.TSMdataGroup{ + groups = append(groups, &datastore.TSMdataGroup{ Combo: currCombo, Instances: currInstances, }) - return nil + return groups, nil } -// getTSAttrGroupsComboOnly populates groups from cols such that each group contains a single, -// unique combination of database values corresponding to cols. Other attributes than those in cols -// have the default value for the type (i.e. "" for string, etc.). -// Returns nil upon success, otherwise error. -func getTSAttrGroupsComboOnly(db *sql.DB, cols []string, groups *[]*datastore.TSMdataGroup) error { +// getTSAttrGroupsComboOnly creates an array of groups from cols such that each group contains a +// single, unique combination of database values corresponding to cols. Other attributes than those +// in cols have the default value for the type (i.e. "" for string, etc.). +// Returns (array of groups, nil) upon success, otherwise (..., error). +func getTSAttrGroupsComboOnly(db *sql.DB, cols []string) ([]*datastore.TSMdataGroup, error) { // query database for unique combinations of cols in time_series, ordered by cols colsS := strings.Join(cols, ",") query := fmt.Sprintf("SELECT DISTINCT %s FROM time_series ORDER BY %s", colsS, colsS) rows, err := db.Query(query) if err != nil { - return fmt.Errorf("db.Query() failed: %v", err) + return nil, fmt.Errorf("db.Query() failed: %v", err) } defer rows.Close() + groups := []*datastore.TSMdataGroup{} + // aggregate rows into groups for rows.Next() { // extract tsMdata from current result row tsMdata, err := scanTsMdata(rows, cols) if err != nil { - return fmt.Errorf("scanTsMdata() failed: %v", err) + return nil, fmt.Errorf("scanTsMdata() failed: %v", err) } // add new group with tsMData as the combo (and leaving the instances // array unset) - *groups = append(*groups, &datastore.TSMdataGroup{Combo: tsMdata}) + groups = append(groups, &datastore.TSMdataGroup{Combo: tsMdata}) } - return nil + return groups, nil } // GetTSAttrGroups ... (see documentation in StorageBackend interface) @@ -312,15 +316,16 @@ func (sbe *PostgreSQL) GetTSAttrGroups(request *datastore.GetTSAGRequest) ( return nil, fmt.Errorf("validateAttrs() failed: %v", err) } - groups := []*datastore.TSMdataGroup{} + var groups []*datastore.TSMdataGroup + var err error if request.IncludeInstances { - if err := getTSAttrGroupsIncInstances(sbe.Db, request.Attrs, &groups); err != nil { - return nil, fmt.Errorf("getTSAGroupsIncInstances() failed: %v", err) + if groups, err = getTSAttrGroupsIncInstances(sbe.Db, request.Attrs); err != nil { + return nil, fmt.Errorf("getTSAttrGroupsIncInstances() failed: %v", err) } } else { - if err := getTSAttrGroupsComboOnly(sbe.Db, request.Attrs, &groups); err != nil { - return nil, fmt.Errorf("getTSAGroupsComboOnly() failed: %v", err) + if groups, err = getTSAttrGroupsComboOnly(sbe.Db, request.Attrs); err != nil { + return nil, fmt.Errorf("getTSAttrGroupsComboOnly() failed: %v", err) } } From 11a52037d36892eca63e9d788a78f09147661c07 Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Tue, 5 Dec 2023 10:07:40 +0100 Subject: [PATCH 15/28] Added note about SQL injection --- datastore/storagebackend/postgresql/gettsattrgroups.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/datastore/storagebackend/postgresql/gettsattrgroups.go b/datastore/storagebackend/postgresql/gettsattrgroups.go index 0e5e47e..ae7c093 100644 --- a/datastore/storagebackend/postgresql/gettsattrgroups.go +++ b/datastore/storagebackend/postgresql/gettsattrgroups.go @@ -208,6 +208,10 @@ func getCombo(tsMdata1 *datastore.TSMetadata, goNames []string) (*datastore.TSMe // getTSAttrGroupsIncInstances creates an array of groups from cols such that each group contains // all instances that match a unique combination of database values corresponding to cols. // All attributes, including those in cols, are set to the actual values found in the database. +// +// NOTE: cols is assumed to be sanitized by validateAttrs, so there is no risk of SQL injection +// in the below query. +// // Returns (array of groups, nil) upon success, otherwise (..., error). func getTSAttrGroupsIncInstances( db *sql.DB, cols []string) ([]*datastore.TSMdataGroup, error) { @@ -279,8 +283,13 @@ func getTSAttrGroupsIncInstances( // getTSAttrGroupsComboOnly creates an array of groups from cols such that each group contains a // single, unique combination of database values corresponding to cols. Other attributes than those // in cols have the default value for the type (i.e. "" for string, etc.). +// +// NOTE: cols is assumed to be sanitized by validateAttrs, so there is no risk of SQL injection +// in the below query. +// // Returns (array of groups, nil) upon success, otherwise (..., error). func getTSAttrGroupsComboOnly(db *sql.DB, cols []string) ([]*datastore.TSMdataGroup, error) { + // query database for unique combinations of cols in time_series, ordered by cols colsS := strings.Join(cols, ",") query := fmt.Sprintf("SELECT DISTINCT %s FROM time_series ORDER BY %s", colsS, colsS) From 1e71b0d30141118a3a4f98a46b8216896ba2b17a Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Wed, 6 Dec 2023 16:59:09 +0100 Subject: [PATCH 16/28] Add migration framework to the docker compose. --- .../storagebackend/postgresql/postgresql.go | 2 +- docker-compose.yml | 22 +++++++++++++------ migrate/Dockerfile | 8 +++++++ .../1701872471_initialise_schema.down.sql | 9 ++++++++ .../1701872471_initialise_schema.up.sql | 14 ++++++++---- 5 files changed, 43 insertions(+), 12 deletions(-) create mode 100644 migrate/Dockerfile create mode 100644 migrate/data/migrations/1701872471_initialise_schema.down.sql rename datastore/ts-init.sql => migrate/data/migrations/1701872471_initialise_schema.up.sql (87%) diff --git a/datastore/storagebackend/postgresql/postgresql.go b/datastore/storagebackend/postgresql/postgresql.go index 3dbb4d3..3e19003 100644 --- a/datastore/storagebackend/postgresql/postgresql.go +++ b/datastore/storagebackend/postgresql/postgresql.go @@ -72,7 +72,7 @@ func NewPostgreSQL() (*PostgreSQL, error) { host := common.Getenv("PGHOST", "localhost") port := common.Getenv("PGPORT", "5433") - user := common.Getenv("PGUSER", "postgres") + user := common.Getenv("PGUSER", "db_user") password := common.Getenv("PGPASSWORD", "mysecretpassword") dbname := common.Getenv("PGDBNAME", "data") diff --git a/docker-compose.yml b/docker-compose.yml index 89ee4cf..a33e049 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -10,19 +10,28 @@ services: volumes: # - ts-data:/home/postgres/pgdata/data # for timescale image - ts-data:/var/lib/postgresql/data # for postgres image - - ./datastore/ts-init.sql:/docker-entrypoint-initdb.d/init.sql environment: - POSTGRES_USER=postgres - POSTGRES_PASSWORD=mysecretpassword - POSTGRES_DB=data restart: on-failure healthcheck: - test: [ "CMD-SHELL", "psql postgresql://postgres:mysecretpassword@localhost/data -c \"SELECT COUNT(*) from OBSERVATION\"" ] + test: [ "CMD-SHELL", "pg_isready", "-U", "${POSTGRES_USER}", "-d", "${POSTGRES_DB}"] interval: 5s timeout: 1s retries: 3 start_period: 30s # Failures in 30 seconds do not mark container as unhealthy + migrate: + build: + context: migrate + volumes: + - ./migrate/data/migrations:/data/migrations + command: ["-path", "/data/migrations", "-database", "postgres://postgres:mysecretpassword@db:5432/data?sslmode=disable", "up"] + depends_on: + db: + condition: service_healthy + store: build: context: datastore @@ -45,11 +54,10 @@ services: retries: 3 start_period: 30s # Failures in 30 seconds do not mark container as unhealthy depends_on: - db: - condition: service_healthy + migrate: + condition: service_completed_successfully api: - hostname: api build: context: . # TODO: Needed to get proto file. Find a better solution dockerfile: api/Dockerfile @@ -96,8 +104,8 @@ services: - DSPORT=50050 - BASE_URL=http://api:8000 depends_on: - store: - condition: service_healthy + loader: + condition: service_completed_successfully volumes: ts-data: diff --git a/migrate/Dockerfile b/migrate/Dockerfile new file mode 100644 index 0000000..6dea5f2 --- /dev/null +++ b/migrate/Dockerfile @@ -0,0 +1,8 @@ +FROM migrate/migrate:4 + +SHELL ["/bin/sh", "-eux", "-o", "pipefail", "-c"] + +# update packages in container +RUN apk update \ + && apk upgrade \ + && rm -rf /var/cache/apk/* diff --git a/migrate/data/migrations/1701872471_initialise_schema.down.sql b/migrate/data/migrations/1701872471_initialise_schema.down.sql new file mode 100644 index 0000000..fce56ac --- /dev/null +++ b/migrate/data/migrations/1701872471_initialise_schema.down.sql @@ -0,0 +1,9 @@ +-- Commented out the statements below as you never want to undo the initialise. +-- DROP TABLE IF EXISTS users; +-- DROP TABLE IF EXISTS observation; +-- DROP TABLE IF EXISTS geo_point; +-- -- not supported yet +-- -- DROP TABLE IF EXISTS geo_polygon; +-- DROP TABLE IF EXISTS time_series; +-- DROP USER IF EXISTS db_user; +-- DROP EXTENSION IF EXISTS postgis; diff --git a/datastore/ts-init.sql b/migrate/data/migrations/1701872471_initialise_schema.up.sql similarity index 87% rename from datastore/ts-init.sql rename to migrate/data/migrations/1701872471_initialise_schema.up.sql index 81b6684..5ffd4d2 100644 --- a/datastore/ts-init.sql +++ b/migrate/data/migrations/1701872471_initialise_schema.up.sql @@ -1,6 +1,6 @@ CREATE EXTENSION IF NOT EXISTS postgis; -CREATE TABLE time_series ( +CREATE TABLE IF NOT EXISTS time_series ( id BIGINT PRIMARY KEY GENERATED ALWAYS AS IDENTITY, -- --- BEGIN metadata fields that usually don't vary with obs time --- @@ -40,7 +40,7 @@ CREATE TABLE time_series ( link_hreflang, link_title) ); -CREATE TABLE geo_point ( +CREATE TABLE IF NOT EXISTS geo_point ( id BIGINT PRIMARY KEY GENERATED ALWAYS AS IDENTITY, point GEOGRAPHY(Point, 4326) NOT NULL UNIQUE ); @@ -48,14 +48,14 @@ CREATE TABLE geo_point ( CREATE INDEX geo_point_idx ON geo_point USING GIST(point); -- not supported yet --- CREATE TABLE geo_polygon ( +-- CREATE TABLE IF NOT EXISTS geo_polygon ( -- id BIGINT PRIMARY KEY GENERATED ALWAYS AS IDENTITY, -- polygon GEOGRAPHY(Polygon, 4326) NOT NULL -- ) -- -- CREATE INDEX geo_polygon_idx ON geo_polygon USING GIST(polygon); -CREATE TABLE observation ( +CREATE TABLE IF NOT EXISTS observation ( ts_id BIGINT NOT NULL REFERENCES time_series(id) ON DELETE CASCADE, -- --- BEGIN metadata fields that usually vary with obs time --- @@ -91,3 +91,9 @@ CREATE TABLE observation ( PRIMARY KEY (ts_id, obstime_instant) ); + +CREATE USER db_user WITH NOSUPERUSER NOCREATEDB NOCREATEROLE PASSWORD 'mysecretpassword'; +ALTER ROLE db_user SET search_path TO public; +GRANT USAGE ON SCHEMA public TO db_user; +GRANT SELECT, INSERT, UPDATE, DELETE ON ALL TABLES IN SCHEMA public TO db_user; +GRANT USAGE, SELECT ON ALL SEQUENCES IN SCHEMA public to db_user; From 92a79ffd749bba4d2cc287767ac708e5e23355d9 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Thu, 7 Dec 2023 12:07:01 +0100 Subject: [PATCH 17/28] Add hack comment to the database healthcheck. --- docker-compose.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index a33e049..a9aca42 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -16,7 +16,8 @@ services: - POSTGRES_DB=data restart: on-failure healthcheck: - test: [ "CMD-SHELL", "pg_isready", "-U", "${POSTGRES_USER}", "-d", "${POSTGRES_DB}"] + # HACK This healthcheck is always healthy + test: [ "CMD-SHELL", "psql postgresql://postgres:mysecretpassword@localhost -XtAc \"SELECT 1 FROM pg_database WHERE datname='data'\"" ] interval: 5s timeout: 1s retries: 3 @@ -104,8 +105,8 @@ services: - DSPORT=50050 - BASE_URL=http://api:8000 depends_on: - loader: - condition: service_completed_successfully + store: + condition: service_healthy volumes: ts-data: From 4ac6f8c6a6e2e864243e760afa70afea0f0d5a70 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Thu, 7 Dec 2023 13:09:52 +0100 Subject: [PATCH 18/28] Add readme with urls that point to migration framework instructions. --- migrate/README.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 migrate/README.md diff --git a/migrate/README.md b/migrate/README.md new file mode 100644 index 0000000..d539f59 --- /dev/null +++ b/migrate/README.md @@ -0,0 +1,8 @@ +# Migration Framework +To have reproducible environments, support rollbacks and make sure that every change is only executed once we use [Golang Migrate](https://github.com/golang-migrate/migrate/tree/master) as migration framework. + +Installation instructions and basic commands can be found here: +https://github.com/golang-migrate/migrate/tree/master/cmd/migrate + +Migration file format instructions can be found here: +https://github.com/golang-migrate/migrate/blob/master/MIGRATIONS.md From 516e6f2ad215f67f6cea08e10692b01884af0864 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Thu, 7 Dec 2023 16:07:39 +0100 Subject: [PATCH 19/28] Postgresql can only be healthy when it does not have to restart anymore. Updated the healtcheck to wait for postgresql to be up for at least a minute. --- docker-compose.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index a9aca42..6adacdb 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -16,8 +16,10 @@ services: - POSTGRES_DB=data restart: on-failure healthcheck: - # HACK This healthcheck is always healthy - test: [ "CMD-SHELL", "psql postgresql://postgres:mysecretpassword@localhost -XtAc \"SELECT 1 FROM pg_database WHERE datname='data'\"" ] + # TODO Healthcheck is to early healthy as postgres exists but is restarted because postgis is installed. +# test: [ "CMD-SHELL", "psql postgresql://postgres:mysecretpassword@localhost -XtAc \"SELECT 1 FROM pg_database WHERE datname='data'\"" ] +# SELECT 1 FROM (SELECT current_timestamp - pg_postmaster_start_time() AS uptime) AS t WHERE t.uptime > interval '1 minute'; + test: [ "CMD-SHELL", "psql postgresql://postgres:mysecretpassword@localhost -XtAc \"SELECT 1 FROM (SELECT current_timestamp - pg_postmaster_start_time() AS uptime) AS t WHERE t.uptime > interval '1 minute'\"" ] interval: 5s timeout: 1s retries: 3 From 4cebbf879849e85218a4bbfc36d9fcd152cabe29 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Thu, 7 Dec 2023 17:42:30 +0100 Subject: [PATCH 20/28] Move healthcheck into docker image. --- database/Dockerfile | 12 ++++++++++++ database/postgis_healthcheck.sh | 14 ++++++++++++++ docker-compose.yml | 9 ++++----- 3 files changed, 30 insertions(+), 5 deletions(-) create mode 100644 database/Dockerfile create mode 100755 database/postgis_healthcheck.sh diff --git a/database/Dockerfile b/database/Dockerfile new file mode 100644 index 0000000..3fec301 --- /dev/null +++ b/database/Dockerfile @@ -0,0 +1,12 @@ +FROM kartoza/postgis:15 + +SHELL ["/bin/bash", "-eux", "-o", "pipefail", "-c"] + +# update packages in container +RUN apt-get update \ + && apt-get -y upgrade \ + && apt-get autoremove -y \ + && apt-get clean \ + && rm -rf /var/lib/apt/lists/* + +COPY postgis_healthcheck.sh / diff --git a/database/postgis_healthcheck.sh b/database/postgis_healthcheck.sh new file mode 100755 index 0000000..d1982ad --- /dev/null +++ b/database/postgis_healthcheck.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash + +CONNECTION_STRING=$1 # Postgres connection string +UPTIME_AMOUNT=${2:-1} # Number of e.g. hours, minutes, seconds +UPTIME_TYPE=${3:-"minute"} # E.g. hour, minute, second + +# Return exit code based on the uptime of postgres +if [[ $(psql "${CONNECTION_STRING}" -XtAc \ + "SELECT COUNT(*) FROM (SELECT current_timestamp - pg_postmaster_start_time() AS uptime) AS t WHERE t.uptime > interval '${UPTIME_AMOUNT} ${UPTIME_TYPE}'") == 1 ]]; +then + exit 0 +else + exit 1 +fi diff --git a/docker-compose.yml b/docker-compose.yml index 6adacdb..b0ab21c 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -4,7 +4,9 @@ name: datastore services: db: # image: timescale/timescaledb-ha:pg15-latest - image: kartoza/postgis:15 # Use this instead of the official image as it has an arm64 image +# image: kartoza/postgis:15 # Use this instead of the official image as it has an arm64 image + build: + context: database ports: - "5433:5432" volumes: @@ -16,10 +18,7 @@ services: - POSTGRES_DB=data restart: on-failure healthcheck: - # TODO Healthcheck is to early healthy as postgres exists but is restarted because postgis is installed. -# test: [ "CMD-SHELL", "psql postgresql://postgres:mysecretpassword@localhost -XtAc \"SELECT 1 FROM pg_database WHERE datname='data'\"" ] -# SELECT 1 FROM (SELECT current_timestamp - pg_postmaster_start_time() AS uptime) AS t WHERE t.uptime > interval '1 minute'; - test: [ "CMD-SHELL", "psql postgresql://postgres:mysecretpassword@localhost -XtAc \"SELECT 1 FROM (SELECT current_timestamp - pg_postmaster_start_time() AS uptime) AS t WHERE t.uptime > interval '1 minute'\"" ] + test: ["CMD-SHELL", "/postgis_healthcheck.sh postgresql://postgres:mysecretpassword@localhost/data 10 second"] interval: 5s timeout: 1s retries: 3 From 0ccff85f1ce3f190280378156fa6cae8958817da Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Fri, 8 Dec 2023 09:37:51 +0100 Subject: [PATCH 21/28] Building the image takes quite long when updating all the dependencies, an alternative can be mounting the healthcheck. --- database/Dockerfile | 12 ------------ docker-compose.yml | 5 ++--- 2 files changed, 2 insertions(+), 15 deletions(-) delete mode 100644 database/Dockerfile diff --git a/database/Dockerfile b/database/Dockerfile deleted file mode 100644 index 3fec301..0000000 --- a/database/Dockerfile +++ /dev/null @@ -1,12 +0,0 @@ -FROM kartoza/postgis:15 - -SHELL ["/bin/bash", "-eux", "-o", "pipefail", "-c"] - -# update packages in container -RUN apt-get update \ - && apt-get -y upgrade \ - && apt-get autoremove -y \ - && apt-get clean \ - && rm -rf /var/lib/apt/lists/* - -COPY postgis_healthcheck.sh / diff --git a/docker-compose.yml b/docker-compose.yml index b0ab21c..f5f276f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -4,14 +4,13 @@ name: datastore services: db: # image: timescale/timescaledb-ha:pg15-latest -# image: kartoza/postgis:15 # Use this instead of the official image as it has an arm64 image - build: - context: database + image: kartoza/postgis:15 # Use this instead of the official image as it has an arm64 image ports: - "5433:5432" volumes: # - ts-data:/home/postgres/pgdata/data # for timescale image - ts-data:/var/lib/postgresql/data # for postgres image + - ./database/postgis_healthcheck.sh:/postgis_healthcheck.sh # for the healthcheck environment: - POSTGRES_USER=postgres - POSTGRES_PASSWORD=mysecretpassword From b6d2a40d25df718f3c7cbc2b4757d8badc83b1b3 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Fri, 8 Dec 2023 10:04:00 +0100 Subject: [PATCH 22/28] Remove local migrate image as it does not add much. --- docker-compose.yml | 3 +-- migrate/Dockerfile | 8 -------- 2 files changed, 1 insertion(+), 10 deletions(-) delete mode 100644 migrate/Dockerfile diff --git a/docker-compose.yml b/docker-compose.yml index f5f276f..b86294b 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -24,8 +24,7 @@ services: start_period: 30s # Failures in 30 seconds do not mark container as unhealthy migrate: - build: - context: migrate + image: migrate/migrate volumes: - ./migrate/data/migrations:/data/migrations command: ["-path", "/data/migrations", "-database", "postgres://postgres:mysecretpassword@db:5432/data?sslmode=disable", "up"] diff --git a/migrate/Dockerfile b/migrate/Dockerfile deleted file mode 100644 index 6dea5f2..0000000 --- a/migrate/Dockerfile +++ /dev/null @@ -1,8 +0,0 @@ -FROM migrate/migrate:4 - -SHELL ["/bin/sh", "-eux", "-o", "pipefail", "-c"] - -# update packages in container -RUN apk update \ - && apk upgrade \ - && rm -rf /var/cache/apk/* From f8bf092bb48ad542c2783a20091664cc51a577f8 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Fri, 8 Dec 2023 10:16:06 +0100 Subject: [PATCH 23/28] Add Hack comment to postgis healthcheck. --- ...{postgis_healthcheck.sh => healthcheck_postgis_uptime.sh} | 0 docker-compose.yml | 5 +++-- 2 files changed, 3 insertions(+), 2 deletions(-) rename database/{postgis_healthcheck.sh => healthcheck_postgis_uptime.sh} (100%) diff --git a/database/postgis_healthcheck.sh b/database/healthcheck_postgis_uptime.sh similarity index 100% rename from database/postgis_healthcheck.sh rename to database/healthcheck_postgis_uptime.sh diff --git a/docker-compose.yml b/docker-compose.yml index b86294b..d1ec933 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -10,14 +10,15 @@ services: volumes: # - ts-data:/home/postgres/pgdata/data # for timescale image - ts-data:/var/lib/postgresql/data # for postgres image - - ./database/postgis_healthcheck.sh:/postgis_healthcheck.sh # for the healthcheck + - ./database/healthcheck_postgis_uptime.sh:/healthcheck_postgis_uptime.sh # for the healthcheck environment: - POSTGRES_USER=postgres - POSTGRES_PASSWORD=mysecretpassword - POSTGRES_DB=data restart: on-failure healthcheck: - test: ["CMD-SHELL", "/postgis_healthcheck.sh postgresql://postgres:mysecretpassword@localhost/data 10 second"] + # HACK Due to the installation of Postgis extension the database is restarted, the healthcheck checks if the database is up for longer than specified time. + test: ["CMD-SHELL", "/healthcheck_postgis_uptime.sh postgresql://postgres:mysecretpassword@localhost/data 10 second"] interval: 5s timeout: 1s retries: 3 From 293ce4fd24bcfc30aa2920b5e61daf81ea740236 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Mon, 11 Dec 2023 15:53:25 +0100 Subject: [PATCH 24/28] Cleanup the migrations. --- .../1701872471_initialise_schema.down.sql | 3 --- .../1701872471_initialise_schema.up.sql | 21 +----------------- .../1702281165_geo_polygon.down.sql | 7 ++++++ .../1702281165_geo_polygon.up.sql | 22 +++++++++++++++++++ 4 files changed, 30 insertions(+), 23 deletions(-) create mode 100644 migrate/data/not_supported_yet/1702281165_geo_polygon.down.sql create mode 100644 migrate/data/not_supported_yet/1702281165_geo_polygon.up.sql diff --git a/migrate/data/migrations/1701872471_initialise_schema.down.sql b/migrate/data/migrations/1701872471_initialise_schema.down.sql index fce56ac..4953ff0 100644 --- a/migrate/data/migrations/1701872471_initialise_schema.down.sql +++ b/migrate/data/migrations/1701872471_initialise_schema.down.sql @@ -1,9 +1,6 @@ -- Commented out the statements below as you never want to undo the initialise. --- DROP TABLE IF EXISTS users; -- DROP TABLE IF EXISTS observation; -- DROP TABLE IF EXISTS geo_point; --- -- not supported yet --- -- DROP TABLE IF EXISTS geo_polygon; -- DROP TABLE IF EXISTS time_series; -- DROP USER IF EXISTS db_user; -- DROP EXTENSION IF EXISTS postgis; diff --git a/migrate/data/migrations/1701872471_initialise_schema.up.sql b/migrate/data/migrations/1701872471_initialise_schema.up.sql index 5ffd4d2..750a89e 100644 --- a/migrate/data/migrations/1701872471_initialise_schema.up.sql +++ b/migrate/data/migrations/1701872471_initialise_schema.up.sql @@ -47,14 +47,6 @@ CREATE TABLE IF NOT EXISTS geo_point ( CREATE INDEX geo_point_idx ON geo_point USING GIST(point); --- not supported yet --- CREATE TABLE IF NOT EXISTS geo_polygon ( --- id BIGINT PRIMARY KEY GENERATED ALWAYS AS IDENTITY, --- polygon GEOGRAPHY(Polygon, 4326) NOT NULL --- ) --- --- CREATE INDEX geo_polygon_idx ON geo_polygon USING GIST(polygon); - CREATE TABLE IF NOT EXISTS observation ( ts_id BIGINT NOT NULL REFERENCES time_series(id) ON DELETE CASCADE, @@ -64,7 +56,6 @@ CREATE TABLE IF NOT EXISTS observation ( -- Refer to geometry via a foreign key to ensure that each distinct geometry is -- stored only once in the geo_* table, thus speeding up geo search. geo_point_id BIGINT NOT NULL REFERENCES geo_point(id) ON DELETE CASCADE, - -- geo_polygon_id integer NOT NULL REFERENCES geo_polygon(id) ON DELETE CASCADE, -- not supported yet pubtime timestamptz NOT NULL, -- required data_id TEXT NOT NULL, -- required @@ -72,19 +63,9 @@ CREATE TABLE IF NOT EXISTS observation ( metadata_id TEXT NOT NULL, -- required -- --- BEGIN for now support only a single instant for obs time --------- - obstime_instant timestamptz, -- NOT NULL, but implied by being part of PK + obstime_instant timestamptz, -- NOT NULL, but implied by being part of PK; obs time variant 1: single instant -- --- END for now support only a single instant for obs time --------- - -- --- BEGIN support both single instant and interval for obs time --------- - -- obstime_instant timestamptz, -- obs time variant 1: single instant - -- obstime_start timestamptz, -- obs time variant 2: interval - -- obstime_end timestamptz, - -- CHECK ( -- ensure exactly one of [1] obstime_instant and [2] obstime_start/-end is defined - -- ((obstime_instant IS NOT NULL) AND (obstime_start IS NULL) AND (obstime_end IS NULL)) OR - -- ((obstime_instant IS NULL) AND (obstime_start IS NOT NULL) AND (obstime_end IS NOT NULL)) - -- ), - -- --- END support both single instant and interval for obs time --------- - processing_level TEXT, value TEXT NOT NULL, -- obs value -- --- END metadata fields that usually vary with obs time --- diff --git a/migrate/data/not_supported_yet/1702281165_geo_polygon.down.sql b/migrate/data/not_supported_yet/1702281165_geo_polygon.down.sql new file mode 100644 index 0000000..2302967 --- /dev/null +++ b/migrate/data/not_supported_yet/1702281165_geo_polygon.down.sql @@ -0,0 +1,7 @@ +ALTER TABLE observation + DROP COLUMN IF EXISTS geo_polygon_id, + DROP COLUMN IF EXISTS obstime_start, -- obs time variant 2: interval + DROP COLUMN IF EXISTS obstime_end, + DROP CONSTRAINT IF EXISTS observation_chk_one_obs_time; + +DROP TABLE IF EXISTS geo_polygon; diff --git a/migrate/data/not_supported_yet/1702281165_geo_polygon.up.sql b/migrate/data/not_supported_yet/1702281165_geo_polygon.up.sql new file mode 100644 index 0000000..478cc49 --- /dev/null +++ b/migrate/data/not_supported_yet/1702281165_geo_polygon.up.sql @@ -0,0 +1,22 @@ +-- not supported yet +CREATE TABLE IF NOT EXISTS geo_polygon ( + id BIGINT PRIMARY KEY GENERATED ALWAYS AS IDENTITY, + polygon GEOGRAPHY(Polygon, 4326) NOT NULL +); + +CREATE INDEX geo_polygon_idx ON geo_polygon USING GIST(polygon); + +------- BEGIN support both single instant and interval for obs time --------- +-- TODO: Fix geo_polygon_id. How to fill the existing rows, otherwise column cannot be added +-- ALTER TABLE observation +-- ADD geo_polygon_id integer NOT NULL REFERENCES geo_polygon(id) ON DELETE CASCADE; -- not supported yet + +ALTER TABLE observation + ADD obstime_start timestamptz, -- obs time variant 2: interval + ADD obstime_end timestamptz, + ADD CONSTRAINT observation_chk_one_obs_time + CHECK ( -- ensure exactly one of [1] obstime_instant and [2] obstime_start/-end is defined + ((obstime_instant IS NOT NULL) AND (obstime_start IS NULL) AND (obstime_end IS NULL)) OR + ((obstime_instant IS NULL) AND (obstime_start IS NOT NULL) AND (obstime_end IS NOT NULL)) + ); +------- END support both single instant and interval for obs time --------- From fb6fb4581b7a56bca30ad086e0ecdc8781ce44ca Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Tue, 12 Dec 2023 18:00:50 +0100 Subject: [PATCH 25/28] Remove IF NOT EXISTS as it is unnecessary. --- migrate/data/migrations/1701872471_initialise_schema.up.sql | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/migrate/data/migrations/1701872471_initialise_schema.up.sql b/migrate/data/migrations/1701872471_initialise_schema.up.sql index 750a89e..2b5f6e6 100644 --- a/migrate/data/migrations/1701872471_initialise_schema.up.sql +++ b/migrate/data/migrations/1701872471_initialise_schema.up.sql @@ -1,6 +1,6 @@ CREATE EXTENSION IF NOT EXISTS postgis; -CREATE TABLE IF NOT EXISTS time_series ( +CREATE TABLE time_series ( id BIGINT PRIMARY KEY GENERATED ALWAYS AS IDENTITY, -- --- BEGIN metadata fields that usually don't vary with obs time --- @@ -40,14 +40,14 @@ CREATE TABLE IF NOT EXISTS time_series ( link_hreflang, link_title) ); -CREATE TABLE IF NOT EXISTS geo_point ( +CREATE TABLE geo_point ( id BIGINT PRIMARY KEY GENERATED ALWAYS AS IDENTITY, point GEOGRAPHY(Point, 4326) NOT NULL UNIQUE ); CREATE INDEX geo_point_idx ON geo_point USING GIST(point); -CREATE TABLE IF NOT EXISTS observation ( +CREATE TABLE observation ( ts_id BIGINT NOT NULL REFERENCES time_series(id) ON DELETE CASCADE, -- --- BEGIN metadata fields that usually vary with obs time --- From f38a0f2e1c62a636d5dd4f6cac244c60a5dfd2b8 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Wed, 13 Dec 2023 09:35:13 +0100 Subject: [PATCH 26/28] Remove user --- datastore/storagebackend/postgresql/postgresql.go | 2 +- .../data/migrations/1701872471_initialise_schema.down.sql | 1 - migrate/data/migrations/1701872471_initialise_schema.up.sql | 6 ------ 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/datastore/storagebackend/postgresql/postgresql.go b/datastore/storagebackend/postgresql/postgresql.go index 3e19003..3dbb4d3 100644 --- a/datastore/storagebackend/postgresql/postgresql.go +++ b/datastore/storagebackend/postgresql/postgresql.go @@ -72,7 +72,7 @@ func NewPostgreSQL() (*PostgreSQL, error) { host := common.Getenv("PGHOST", "localhost") port := common.Getenv("PGPORT", "5433") - user := common.Getenv("PGUSER", "db_user") + user := common.Getenv("PGUSER", "postgres") password := common.Getenv("PGPASSWORD", "mysecretpassword") dbname := common.Getenv("PGDBNAME", "data") diff --git a/migrate/data/migrations/1701872471_initialise_schema.down.sql b/migrate/data/migrations/1701872471_initialise_schema.down.sql index 4953ff0..ba13745 100644 --- a/migrate/data/migrations/1701872471_initialise_schema.down.sql +++ b/migrate/data/migrations/1701872471_initialise_schema.down.sql @@ -2,5 +2,4 @@ -- DROP TABLE IF EXISTS observation; -- DROP TABLE IF EXISTS geo_point; -- DROP TABLE IF EXISTS time_series; --- DROP USER IF EXISTS db_user; -- DROP EXTENSION IF EXISTS postgis; diff --git a/migrate/data/migrations/1701872471_initialise_schema.up.sql b/migrate/data/migrations/1701872471_initialise_schema.up.sql index 2b5f6e6..dd7ae0c 100644 --- a/migrate/data/migrations/1701872471_initialise_schema.up.sql +++ b/migrate/data/migrations/1701872471_initialise_schema.up.sql @@ -72,9 +72,3 @@ CREATE TABLE observation ( PRIMARY KEY (ts_id, obstime_instant) ); - -CREATE USER db_user WITH NOSUPERUSER NOCREATEDB NOCREATEROLE PASSWORD 'mysecretpassword'; -ALTER ROLE db_user SET search_path TO public; -GRANT USAGE ON SCHEMA public TO db_user; -GRANT SELECT, INSERT, UPDATE, DELETE ON ALL TABLES IN SCHEMA public TO db_user; -GRANT USAGE, SELECT ON ALL SEQUENCES IN SCHEMA public to db_user; From f124e067b629f04148412da1690facdf84062ae1 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Wed, 13 Dec 2023 12:08:58 +0100 Subject: [PATCH 27/28] Pin docker image on major version. --- docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index d1ec933..b406de2 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -25,7 +25,7 @@ services: start_period: 30s # Failures in 30 seconds do not mark container as unhealthy migrate: - image: migrate/migrate + image: migrate/migrate:4 volumes: - ./migrate/data/migrations:/data/migrations command: ["-path", "/data/migrations", "-database", "postgres://postgres:mysecretpassword@db:5432/data?sslmode=disable", "up"] From fd55f15ac1c12b47a50433e4a18577716d06f158 Mon Sep 17 00:00:00 2001 From: Jeffrey Vervoort Date: Wed, 13 Dec 2023 13:00:07 +0100 Subject: [PATCH 28/28] Add information about the implementation of the migrations to the readme. --- migrate/README.md | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/migrate/README.md b/migrate/README.md index d539f59..7e4353e 100644 --- a/migrate/README.md +++ b/migrate/README.md @@ -1,8 +1,28 @@ # Migration Framework -To have reproducible environments, support rollbacks and make sure that every change is only executed once we use [Golang Migrate](https://github.com/golang-migrate/migrate/tree/master) as migration framework. +To have reproducible environments, support rollbacks and that every change is only executed once, we use [Golang Migrate](https://github.com/golang-migrate/migrate/tree/master) as a migration framework. -Installation instructions and basic commands can be found here: +See the following URL for installation instructions and basic commands: https://github.com/golang-migrate/migrate/tree/master/cmd/migrate -Migration file format instructions can be found here: +See the following URL for the migration file format instructions: https://github.com/golang-migrate/migrate/blob/master/MIGRATIONS.md + +## Practicalities +### Initialisation +The migration framework initialises the database. Therefore, no database tables exist before running the migrate step in the docker compose. + +### File name format +The migration file name format follows the suggestion in [MIGRATIONS.md](https://github.com/golang-migrate/migrate/blob/master/MIGRATIONS.md) to use a timestamp as version. + +``` +{version}_{title}.up.{extension} +{version}_{title}.down.{extension} +``` + +On Linux, you can retrieve the current timestamp by running: `date +%s`. + + +### Migration Path +The path `./migrate/data/migrations` is mounted on the migrate container. Thus, the docker container only executes the migrations in this path. + +The other path: `./migrate/data/not_supported_yet`, contains an example migration based on code comments about unfinished work from the initialise script. As the path is not mounted, the docker container does not execute migrations in that path. To try out the migrations move the files to `./migrate/data/migrations`.