ObservedProperty label & test data loader through ingestion

[fjugipe]

• Change ObservedProperty label to standard_name without underscores
• Change Parameter description to have standard_name without underscores
• Update test data to be on par with current ingest
• Add a data loader to load test data through ingest
• Add Dockerized unit tests for ingest
• Update ci.yml to load test data through ingest and
• Comment test coverage for ingest unit tests
• Add new just commands

closes #204, closes #190

[github-actions]

Unit Test Coverage Report

File	Stmts	Miss	Cover	Missing
__init__.py	0	0	100%
datastore_pb2.py	61	48	21%	34–81
datastore_pb2_grpc.py	54	27	50%	15–16, 19, 65–80, 121–123, 128–130, 135–137, 142–144, 148–173, 219, 246, 273, 300
export_metrics.py	10	0	100%
grpc_getter.py	20	11	45%	15–19, 23–26, 30–32, 36–38
locustfile.py	15	15	0%	1–31
main.py	43	7	84%	45, 50, 60, 70–71, 81–82
metadata_endpoints.py	66	32	52%	45–54, 58, 85, 100–220, 224
response_classes.py	5	0	100%
utilities.py	174	46	74%	20, 38, 45, 67–70, 78–89, 94–101, 121, 125, 127, 155, 161, 179, 193–194, 198, 214–218, 222–228, 232–234, 264, 268, 290, 295
custom_geo_json
edr_feature_collection.py	6	0	100%
formatters
__init__.py	11	0	100%
covjson.py	60	1	98%	91
geojson.py	21	2	90%	27, 52
openapi
custom_dimension_examples.py	4	0	100%
edr_query_parameter_descriptions.py	11	0	100%
openapi_examples.py	13	0	100%
routers
__init__.py	0	0	100%
edr.py	101	4	96%	348–349, 438–439
feature.py	47	19	60%	99–132, 148–153, 159–181
TOTAL	722	212	71%

Title	Coverage	Tests	Skipped	Failures	Errors	Time
API Unit Tests		30	0 💤	0 ❌	0 🔥	1.798s ⏱️
Ingest Unit Tests		16	0 💤	0 ❌	0 🔥	1m 41s ⏱️

[lukas-phaf]

It is not clear to me why test-ingest is a separate job. I think it leads to a confusing mix of jobs and steps in the action output.

Also, the matrix of python versions leads to additional confusion, especially as it has only one version. Can we take the blue pill and comment out the whole strategy bit for now so the matrix doesn't show up?

To be honest, I also don't see the advantage of a separate publish-test-results job. It was added by @Jeffrey-Vervoort-KNMI as separation of concern. But it also adds a lot of noise (for example to retrieve the artifacts). I don't think the contents and issues permissions are needed, so I would be fine with all the main job having pull-requests permissions.

[fjugipe]

I see the point on separate jobs for tests when you trigger them for example on push to that service. Currently we don't have that, but should we? Running performance tests to the api on each push, when changes are made to ingest seems a bit unnecessary in my opinion and I personally would run them only on pull_request.

I agree with you on the changes, especially the strategy is unnecessary now that the ingest tests are run in a container. Overall I think that our workflows could have a slight rework. But in this PR I changed only minor things, since it is for another issue.

I can continue refactoring the workflows here, but I think the work should be done in a separate branch, since this PR contains changes which affect how we present the data.

[lukas-phaf]

Great improvements!

[lukas-phaf]

Agreed that the worflows refactor should be in another PR. From: Jukka Pelli ***@***.***> Reply-To: EURODEO/e-soh ***@***.***> Date: Wednesday, October 23, 2024 at 13:46 To: EURODEO/e-soh ***@***.***> Cc: "Phaf, Lukas (KNMI)" ***@***.***>, Review requested ***@***.***> Subject: Re: [EURODEO/e-soh] ObservedProperty label & test data loader through ingestion (PR #209) I see the point on separate jobs for tests when you trigger them for example on push to that service. Currently we don't have that, but should we? Running performance tests to the api on each push, when changes are made to ingest seems a bit unnecessary in my opinion and I personally would run them only on pull_request. I agree with you on the changes, especially the strategy is unnecessary now that the ingest tests are run in a container. Overall I think that our workflows could have a slight rework. But in this PR I changed only minor things, since it is for another issue. I can continue refactoring the workflows here, but I think the work should be done in a separate branch, since this PR contains changes which affect how we present the data. — Reply to this email directly, view it on GitHub<#209 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AF2G54THNODJ3K5CUBOSA33Z46EA3AVCNFSM6AAAAABQGB2NKKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZRHA2TEOBSGM>. You are receiving this because your review was requested.Message ID: ***@***.***>

[Jeffrey-Vervoort-KNMI]

The main reason was minimal principle. The other parts of the jobs did not need write permission. Looking at the documentation again, I think that as Lukas says, the permissions for `issues` and `contents` can be removed.

…

________________________________ From: Lukas Phaf ***@***.***> Sent: 23 October 2024 11:50:39 To: EURODEO/e-soh Cc: Vervoort, Jeffrey; Mention Subject: Re: [EURODEO/e-soh] ObservedProperty label & test data loader through ingestion (PR #209) It is not clear to me why test-ingest is a separate job. I think it leads to a confusing mix of jobs and steps in the action output. Also, the matrix of python versions leads to additional confusion, especially as it has only one version. Can we take the blue pill and comment out the whole strategy bit for now so the matrix doesn't show up? To be honest, I also don't see the advantage of a separate publish-test-results job. It was added by @Jeffrey-Vervoort-KNMI<https://github.com/Jeffrey-Vervoort-KNMI> as separation of concern. But it also adds a lot of noise (for example to retrieve the artifacts). I don't think the contents and issues permissions are needed<https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/controlling-permissions-for-github_token>, so I would be fine with all the main job having pull-requests permissions. — Reply to this email directly, view it on GitHub<#209 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ATJG55DPQABDRRNXHTWSFH3Z45WO7AVCNFSM6AAAAABQGB2NKKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZRGU2TMNZYGI>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[lukas-phaf]

I noticed this warning when running just load, which seems a bit worrying, but I don't seem to be able to pin it down:

/usr/local/lib/python3.11/site-packages/google/protobuf/runtime_version.py:112: UserWarning: Protobuf gencode version 5.27.2 is older than the runtime version 5.28.2 at datastore.proto. Please avoid checked-in Protobuf gencode that can be obsolete.
  warnings.warn(

Edit: Oh, it looks like this a common issue...