improve STAC OpenAPI and service metadata #621
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
documentation
Collaborator
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/3907/Result ❌ FAILUREBIRDHOUSE_DEPLOY_BRANCH : stac-service-metadata DACCS_IAC_BRANCH : master DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-216.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/583/NOTEBOOK TEST RESULTS |
mishaschwartz
requested changes
Dec 19, 2025
Collaborator
mishaschwartz
left a comment
mishaschwartz
left a comment
There was a problem hiding this comment.
Looks good! Just one small change...
| \$STAC_VERSION | ||
| \$STAC_IMAGE | ||
| \$STAC_IMAGE_URI | ||
| \$STAC_LICENSE_URL |
Collaborator
There was a problem hiding this comment.
These should go in "VARS" since they'll break things if they're not set
(service-config.json will contain invalid URLs and the OPENAPI_URL and DOCS_URL paths will just be /stac which may confuse the stac-app)
Member
Author
There was a problem hiding this comment.
I though VARS is for "you must provide them" and OPTIONAL_VARS for "some default exists", and not overriding them will use the defaults. If overridden with inadequate values, it can break, and it is up to the dev not to provide garbage in.
If that's not the case, then there are many actually breaking OPTIONAL_VARS all over the repository, such as all _DOCKER, _VERSION and _IMAGE (and many more) that create invalid docker compose if explicitly set empty.
I think in this specific case, setting them to empty explicitly (for whatever reason) would still startup correctly, though indeed would lead to weird URLs.
Should the VARS/OPTIONAL_VARS be better defined, with review of existing definitions? It is not the first time this confusion occurred. The current descriptions leave them up to personal interpretation, notably because both can use defaults, "they do not have to be defined in 'env.local' if values are adequate", and empty is technically a defined variable, just not "set".
birdhouse-deploy/birdhouse/default.env
Lines 48 to 51 in 2ef86b1
birdhouse-deploy/birdhouse/default.env
Lines 70 to 74 in 2ef86b1
Collaborator
There was a problem hiding this comment.
The error message when a VARS variable is empty is: "Required variable ${v#$} is not set" and then the process exits. This is checked with the -z test which checks for empty or unset strings.
To me this has always implied that these variables must be set to a non-empty value in order for the stack to work properly.
I though VARS is for "you must provide them" and OPTIONAL_VARS for "some default exists",
I think that this is almost right but also it means that you cannot override the default VARS variables and make the value empty.
If that's not the case, then there are many actually breaking OPTIONAL_VARS all over the repository,
This is true as well
Should the VARS/OPTIONAL_VARS be better defined, with review of existing definitions?
Yes please
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Improve STAC OpenAPI and service metadata.
Changes
Non-breaking changes
STAC API: Improve reported service links.
STAC_LICENSE_URLto define the relevant license metadata location of the selected implementation.STAC_OPENAPI_SPEC_PATHandSTAC_OPENAPI_DOCS_PATHto define endpoints of OpenAPI specification./services/stacresponse to provide more metadata links, including license details and betterAPI metadata references. Notably, replace the generic STAC API Core OpenAPI definition by the
implementation-specific definition self-served by the selected docker image to document API extensions.
Breaking changes
CI Operations
birdhouse_daccs_configs_branch: master
birdhouse_skip_ci: false