add node services URI and fields #445

fmigneault · 2024-04-05T21:27:11Z

Overview

Add node services URI and fields.

Changes

Non-breaking changes

Node Services: Add definitions and variables for every service represented by
the DACCS-Climate/Marble-node-registry.
- Add version field using the corresponding <SERVICE>_VERSION variables.
- Add types field restricted by specific values instead of previous keywords expected to be extendable.
- Add <SERVICE>_IMAGE_URI variables to provide rel: service-meta link for every service.

Breaking changes

n/a

Related Issue / Discussion

Resolves 💡 [Feature] Provide component service version #441
Depends on update service schema definitions DACCS-Climate/Marble-node-registry#40 to enforce validation

CI Operations

birdhouse_daccs_configs_branch: master
birdhouse_skip_ci: false

…ypes' field

mishaschwartz

Do we also want to update deprecated-components/flyingpigeon as well?

I'm very happy to not update deprecated components but I just wanted to double check that you left it out intentionally.

mishaschwartz · 2024-04-08T13:21:47Z

birdhouse/components/finch/service-config.json.template

+  "version": "${FINCH_VERSION}",
+  "types": [
+    "wps"
+  ],
  "keywords": [


should we leave these keywords as is if we're moving this information to "types". What if we borrowed some of the tags and categories from canarie-api instead. So for finch we could do something like:

"keywords": ["Climatology", "Cloud", "Data Slicer", "Climate Indices", ...]

Yes. I would like to do this, but until there is a clear-cut version I can refer to, adding these to the keywords will fail schema validation.

Got it. Try now with version 1.2.0?

Are we going to do this in this PR or in a different one?

If possible, I'd prefer to work on this only once.

birdhouse/components/finch/service-config.json.template

birdhouse/components/raven/docker-compose-extra.yml

tlvu

Ensure template expansion actually work.

Also I do not think this PR introduce a breaking change (in reference to this PR description). The new var has a default value so upgrading to this PR is 100% seamless, no manual intervention required, so nothing "breaking" as per deployment as I can see.

I defer to Misha for review of the content of the .json .

Agree with Misha this PR unified the way to specify all the images so it's a very nice side-effect.

Also, it seems all we need are additional copy/paste to make deprecated-components work so probably cheap enough to make it work as well.

birdhouse/components/finch/service-config.json.template

mishaschwartz · 2024-04-08T18:30:36Z

@tlvu

Also, it seems all we need are additional copy/paste to make deprecated-components work so probably cheap enough to make it work as well.

Cheap enough for sure, but can we also plan to stop updating these at some point? I mean, at what point do we actually treat them as deprecated instead of just in another folder that happens to be called deprecated-components?

I guess we do not validate the value in "href", that's why we did not caught the wrong template expansion?

We are, it's just that the lnks schema uses the uri-template string type to check the href value and ${some_string_here} is a valid uri-template according to RFC6570. There's not much we can do about that.

tlvu · 2024-04-08T21:13:01Z

at what point do we actually treat them as deprecated instead of just in another folder that happens to be called deprecated-components?

I guess at the point where it's expensive. I agree "expensive" is very vague so up to you I guess.

fmigneault · 2024-04-08T21:29:16Z

I did omit flyingpigeon on purpose due to deprecated, but as highlighted, it is fairly easy to update, so I can add it as well.
Will wait for a release that integrates the relevant updates to the schema to update the references.

fmigneault · 2024-04-08T21:44:59Z

We are, it's just that the lnks schema uses the uri-template string type to check the href value and ${some_string_here} is a valid uri-template according to RFC6570. There's not much we can do about that.

I don't think this is the cause.
According to JSON-schema format is just a hint, not an enforced validation, unless otherwise specified.

jsonschema.validate("^^^!!!! <<SO NOT AN URI>> ???", {"type": "string", "format": "uri-template"})  # OK

Looking up the format check in the validator confirms this (uri and uri-template not checked):

jsonschema.draft202012_format_checker
<FormatChecker checkers=['date', 'email', 'idn-email', 'idn-hostname', 'ipv4', 'ipv6', 'regex', 'uuid']>

So, the real fix would be to add a "pattern": "^https?://" explicitly.

mishaschwartz · 2024-04-09T00:27:04Z

According to JSON-schema format is just a hint, not an enforced validation, unless otherwise specified.

Ok that's good to know....

birdhouse/components/finch/default.env

…birdhouse-deploy into add-service-versions

birdhouse/components/finch/default.env

tlvu · 2024-04-09T16:40:11Z

birdhouse/components/finch/service-config.json.template

+    {
+      "rel": "service-meta",
+      "type": "application/vnd.oci.image.index.v1+json",
+      "href": "${FINCH_IMAGE_URI}"


Just my 2 cents, since the URI, do not start with http, should we name the new field "uri" instead of "href"? "href" would suggest starting with http usually. Ignore if it has to be "href" for compat or other reasons.

"href" is required by the schema so we can't change the name

A non-http scheme value is a valid URI. It uniquely identifies the resource.
I would prefer to have a docker:// scheme or similar to be more explicit if permitted, but docker does not seem to like it. However, the values provided in the defaults can be used directly (e.g. docker pull 'registry.hub.docker.com/pavics/weaver:5.0.0' works, but http://registry.hub.docker.com/pavics/weaver:5.0.0 doesn't), so I find them more useful although maybe counter-intuitive.

Maybe one way to avoid confusion would be to use index.docker.io/pavics/weaver:5.0.0 instead, since it does not redirect to https://hub.docker.com/ like registry.hub.docker.com does? This alternate location still woks with docker pull 'index.docker.io/pavics/weaver:5.0.0', and it looks slightly more like the type definition. The idea of this link is really to have the exact value that can be plugged in the docker-compose service image field.

…e contents with resolved defaults + use '' from service reference instead of global one

tests/test_deployment.py

tests/requirements.txt

mishaschwartz · 2024-04-10T13:11:20Z

birdhouse/scripts/get-services-json.include.sh

+  # read and strip leading/trailing whitespaces
+  SERVICE_CONF="$(cat "${adir}/service-config.json" | sed -z 's/^\s*//;s/\s*$//')"
+  # merge whether config is an array or a single service
+  if [ "$(echo "${SERVICE_CONF}" | head -n 1)" = "[" ]; then


If you just left the stac/service-config.json.template file as is, all of this wouldn't be necessary.

I don't understand why this is needed

Either this, or we patch everywhere that tries to load the JSON.
I add errors when trying to extract the $schema in the tests.

This should be the only place that tries to load the JSON.
The individual files are not meant to be read by anywhere else, their only purpose is to be concatenated by this script so that they can be displayed as one big list.

One of the reasons we had the files as they were before was so that we didn't have to complicate loading them into one big JSON list like this.

However, if we're going to got this route, I'd be in favour of using a proper json parsing tool like jq instead of hacking together JSON snippets with sed. jq has a docker image that we can use if necessary.

I purposely went without the jq approach to avoid introducing this extra dependency since the merge is relatively simple in this case.

One issue with the pseudo-JSON is that it is not obvious (for others) that the incorrectly formed content is intentional to work only once merged. Especially when working with IDEs, or investigating the generated JSON on the server, we get flagged about invalid JSON, which doesn't help maintain them. Since there is a way to make them valid JSON definitions and still work for the aggregation, I would favor using a valid list.

birdhouse/components/thredds/default.env

birdhouse/components/stac/service-config.json.template

mishaschwartz · 2024-04-10T13:22:25Z

birdhouse/components/finch/service-config.json.template

+  "version": "${FINCH_VERSION}",
+  "types": [
+    "wps"
+  ],
  "keywords": [


Are we going to do this in this PR or in a different one?

mishaschwartz · 2024-04-10T13:31:02Z

birdhouse/components/finch/service-config.json.template

  "name": "finch",
+  "version": "${FINCH_VERSION}",
+  "types": [


This hasn't been implemented yet in the schema. Adding this won't cause schema validation to fail but do we plan on updating the schema before or after this PR is merged?

If the schema gets updated to include it beforehand, then we can bump its version right away. Otherwise, it can be done later. As mentioned, it doesn't cause an issue regardless at the moment since it is not validated.

birdhouse/components/hummingbird/default.env

tlvu

LGTM. I have not done a deployment test to ensure all template expansions actually work. I also defer to @mishaschwartz to validate the content of the various .json files and the test.

tlvu · 2024-04-10T13:35:44Z

birdhouse/components/stac/default.env

+# (see: https://github.com/crim-ca/stac-app/blob/40cad1aa7a094d58fca2d3184182761e248f781d/Dockerfile#L15-L20)
+# which corresponds to 2.4.3 release
+export STAC_VERSION=2.4.3-crim-main
+export STAC_IMAGE='ghcr.io/crim-ca/stac-app:main'


Just curious, why not push the image with the actual tag 2.4.3-crim instead of the generic tag main?

See #371 and #346

Yes, I could temporarily push a tag to make the reference more "semver"-like, but like @mishaschwartz highlighted, I would much rather have some work done on using the official image. The one tagged is already 9 months old. I don't want this patch tag to be considered a maintained revision.

tlvu · 2024-04-10T13:37:01Z

birdhouse/components/stac/default.env

+# which is some commits ahead of 'v3.0.0-beta.5' (and multiple behind latest official releases)
+# version name is slightly tweaked to fulfill schema while leaving an obvious trace
+export STAC_BROWSER_VERSION=3.0.0-beta.5-crim-docker-image-push
+export STAC_BROWSER_IMAGE='ghcr.io/crim-ca/stac-browser:docker_image_push'


Same here, 3.0.0-beta.5-crim instead of docker_image_push. It is allowed to tag while on a branch.

…stencies

…IMAGE

fmigneault · 2024-04-15T15:23:03Z

run tests

mishaschwartz

Looks good! Just double checking if there's still more keywords to add or not (see comment here: #445 (comment))

fmigneault · 2024-04-17T18:31:33Z

For the moment, 1.2.0 still refers to keywords as only a specific set of values. Do you want to push a 1.3.0 right away with updated definitions allowing any value, and using types instead for the specific service type allowed?

mishaschwartz · 2024-04-17T19:04:54Z

Right, we're still discussing it here: DACCS-Climate/Marble-node-registry#39

Ok nevermind then

add node services URI with 'rel:service-meta', 'version' field and 't…

41d6be8

…ypes' field

fmigneault self-assigned this Apr 5, 2024

fmigneault requested review from tlvu and mishaschwartz as code owners April 5, 2024 21:27

mishaschwartz requested changes Apr 8, 2024

View reviewed changes

tlvu requested changes Apr 8, 2024

View reviewed changes

birdhouse/components/finch/service-config.json.template Outdated Show resolved Hide resolved

add missing OPTIONAL_VARGS + apply service config for flyingpigeon

2310410

tlvu reviewed Apr 9, 2024

View reviewed changes

birdhouse/components/finch/default.env Outdated Show resolved Hide resolved

mishaschwartz and others added 4 commits April 9, 2024 09:12

Merge branch 'master' into add-service-versions

b6e3b5b

add service VERSION and VERSION_URI variables to DELAYED_EVAL

7522c40

update all service node schemas to 1.2.0

40126f7

Merge branch 'add-service-versions' of https://github.com/bird-house/…

f8b8357

…birdhouse-deploy into add-service-versions

tlvu reviewed Apr 9, 2024

View reviewed changes

birdhouse/components/finch/default.env Outdated Show resolved Hide resolved

tlvu reviewed Apr 9, 2024

View reviewed changes

fix DELAYED_EVAL variable quotes + adjust tests to explicitly validat…

07f6b97

…e contents with resolved defaults + use '' from service reference instead of global one

github-actions bot added ci/deployment Related to deployment utilities and scripts ci/operations Continuous Integration components labels Apr 10, 2024

github-actions bot added the ci/tests Issues or changes related to tests scripts label Apr 10, 2024

fmigneault requested review from tlvu and mishaschwartz April 10, 2024 02:26

mishaschwartz requested changes Apr 10, 2024

View reviewed changes

tlvu approved these changes Apr 10, 2024

View reviewed changes

fmigneault added 3 commits April 10, 2024 15:42

cleanup test requirements + patch missing quotes/DELAYED_EVAL inconsi…

485580a

…stencies

use service-config lists everywhere

9b0753f

fix optional-components/testthredds needing DELAYED_EVAL for THREDDS_…

546f9ac

…IMAGE

fmigneault requested a review from mishaschwartz April 15, 2024 15:18

adjust canarie versions/tag mixup

3b18f57

mishaschwartz approved these changes Apr 17, 2024

View reviewed changes

Bump version: 2.1.3 → 2.2.0

5249a6b

fmigneault merged commit 5a54a1f into master Apr 18, 2024
3 of 4 checks passed

fmigneault deleted the add-service-versions branch April 18, 2024 15:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add node services URI and fields #445

add node services URI and fields #445

fmigneault commented Apr 5, 2024 •

edited

Loading

mishaschwartz left a comment

mishaschwartz Apr 8, 2024

fmigneault Apr 8, 2024

mishaschwartz Apr 9, 2024

mishaschwartz Apr 10, 2024

fmigneault Apr 10, 2024

tlvu left a comment

mishaschwartz commented Apr 8, 2024

tlvu commented Apr 8, 2024

fmigneault commented Apr 8, 2024

fmigneault commented Apr 8, 2024

mishaschwartz commented Apr 9, 2024

tlvu Apr 9, 2024

mishaschwartz Apr 9, 2024

fmigneault Apr 10, 2024

mishaschwartz Apr 10, 2024

fmigneault Apr 10, 2024

mishaschwartz Apr 15, 2024

fmigneault Apr 15, 2024

mishaschwartz Apr 10, 2024

mishaschwartz Apr 10, 2024

fmigneault Apr 10, 2024

tlvu left a comment

tlvu Apr 10, 2024

mishaschwartz Apr 10, 2024

fmigneault Apr 10, 2024

tlvu Apr 10, 2024

fmigneault commented Apr 15, 2024

mishaschwartz left a comment

fmigneault commented Apr 17, 2024

mishaschwartz commented Apr 17, 2024

add node services URI and fields #445

add node services URI and fields #445

Conversation

fmigneault commented Apr 5, 2024 • edited Loading

Overview

Changes

Related Issue / Discussion

CI Operations

mishaschwartz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tlvu left a comment

Choose a reason for hiding this comment

mishaschwartz commented Apr 8, 2024

tlvu commented Apr 8, 2024

fmigneault commented Apr 8, 2024

fmigneault commented Apr 8, 2024

mishaschwartz commented Apr 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tlvu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmigneault commented Apr 15, 2024

mishaschwartz left a comment

Choose a reason for hiding this comment

fmigneault commented Apr 17, 2024

mishaschwartz commented Apr 17, 2024

fmigneault commented Apr 5, 2024 •

edited

Loading