-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add node services URI and fields #445
Changes from 7 commits
41d6be8
2310410
b6e3b5b
7522c40
40126f7
f8b8357
07f6b97
485580a
9b0753f
546f9ac
3b18f57
5249a6b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,10 @@ | ||
{ | ||
"$schema": "https://raw.githubusercontent.com/DACCS-Climate/Marble-node-registry/main/node_registry.schema.json#service", | ||
"$schema": "https://raw.githubusercontent.com/DACCS-Climate/Marble-node-registry/1.2.0/node_registry.schema.json#service", | ||
"name": "finch", | ||
"version": "${FINCH_VERSION}", | ||
"types": [ | ||
"wps" | ||
], | ||
"keywords": [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. Try now with version 1.2.0? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we going to do this in this PR or in a different one? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If possible, I'd prefer to work on this only once. |
||
"service-wps" | ||
], | ||
|
@@ -20,6 +24,11 @@ | |
"rel": "service-desc", | ||
"type": "text/xml", | ||
"href": "https://${PAVICS_FQDN_PUBLIC}${TWITCHER_PROTECTED_PATH}/finch?service=WPS&request=GetCapabilities" | ||
}, | ||
{ | ||
"rel": "service-meta", | ||
"type": "application/vnd.oci.image.index.v1+json", | ||
"href": "${FINCH_IMAGE_URI}" | ||
fmigneault marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A non-http scheme value is a valid URI. It uniquely identifies the resource. Maybe one way to avoid confusion would be to use |
||
} | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,20 @@ export STAC_POSTGRES_PASSWORD=${POSTGRES_PAVICS_PASSWORD} | |
export STAC_PGUSER=${POSTGRES_PAVICS_USERNAME} | ||
export STAC_PGPASSWORD=${POSTGRES_PAVICS_PASSWORD} | ||
|
||
# 'main' branch points at https://github.com/stac-utils/stac-fastapi/commit/d53e792 | ||
# (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' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just curious, why not push the image with the actual tag There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
export STAC_IMAGE_URI='${STAC_IMAGE}' | ||
|
||
# 'docker_image_push' branch points at https://github.com/crim-ca/stac-browser/tree/docker_image_push | ||
# 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' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, |
||
export STAC_BROWSER_IMAGE_URI='${STAC_BROWSER_IMAGE}' | ||
|
||
# add any new variables not already in 'VARS' or 'OPTIONAL_VARS' that must be replaced in templates here | ||
# single quotes are important in below list to keep variable names intact until 'pavics-compose' parses them | ||
EXTRA_VARS=' | ||
|
@@ -14,3 +28,21 @@ EXTRA_VARS=' | |
# extend the original 'VARS' from 'birdhouse/pavics-compose.sh' to employ them for template substitution | ||
# adding them to 'VARS', they will also be validated in case of override of 'default.env' using 'env.local' | ||
VARS="$VARS $EXTRA_VARS" | ||
|
||
export DELAYED_EVAL=" | ||
$DELAYED_EVAL | ||
STAC_IMAGE | ||
STAC_IMAGE_URI | ||
STAC_BROWSER_IMAGE | ||
STAC_BROWSER_IMAGE_URI | ||
" | ||
|
||
OPTIONAL_VARS=" | ||
$OPTIONAL_VARS | ||
\$STAC_VERSION | ||
\$STAC_IMAGE | ||
\$STAC_IMAGE_URI | ||
\$STAC_BROWSER_VERSION | ||
\$STAC_BROWSER_IMAGE | ||
\$STAC_BROWSER_IMAGE_URI | ||
" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.