add optional-components/stac-data-proxy
#403
Merged
Conversation
This file contains 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
fmigneault
requested review from
huard,
mishaschwartz,
tlvu,
ChaamC and
francisPLT
fmigneault
changed the title
github-actions
bot
added
component/magpie
fmigneault
changed the title
optional-components/stac-data-proxy
fmigneault
added
security
|
@ahandan @ChaamC @francisPLT @Nazim-crim |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2244/Result : failure BIRDHOUSE_DEPLOY_BRANCH : stac-data-proxy DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-133.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1417/NOTEBOOK TEST RESULTS |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2245/Result : failure BIRDHOUSE_DEPLOY_BRANCH : stac-data-proxy DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-133.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1418/NOTEBOOK TEST RESULTS |
|
run tests |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2252/Result : failure BIRDHOUSE_DEPLOY_BRANCH : stac-data-proxy DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1422/NOTEBOOK TEST RESULTS |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2286/Result : success BIRDHOUSE_DEPLOY_BRANCH : stac-data-proxy DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1426/NOTEBOOK TEST RESULTS |
|
@tlvu If everything looks good, that will help me synchronize merges with other STAC-related PRs to avoid conflicts. |
|
I am not able to see how this feature meshes with the functionality for controlling the mount points of THREDDS data, and support for multiple THREDDS catalog that was recently provided in #359. The variables |
|
@dchandan An example would be the GeoTiff, GeoJSON and PNG asset files under https://github.com/ai-extensions/stac-data-loader/tree/main/data/EuroSAT that are referenced by the STAC Collection/Items. At the moment, it is already possible to push those STAC definitions (eg: https://hirondelle.crim.ca/stac/collections/EuroSAT-subset-train), but the assets they refer to must make use of an external host source (temporarily using GitHub raw content endpoint for now, which is not optimal). If the node is a data provider, it should have a way to host its own data without necessarily relying on an external hosting service. |
|
I see. But then why not put that data on THREDDS on the node itself? In your example, it would mean putting the asset files from EuroSAT onto hirondelle's THREDDS catalog. |
|
THREDDS makes sense in the case of netCDF and NcML since there are built-in visualization and formatting services for them, but it is an additional complexity layer for simple EO imagery and other potential data sources that could be represented by STAC. |
|
Maybe something like |
I sort of agree with @dchandan here. Is this new URL and location on disk meant only for STAC data? From the implementation, it looks pretty generic so it could be used for all sort of static data, not just STAC data. So, in the future, if we need for "some other kind of data" we won't need to again change the config, or to go through PRs review again. It's like the new ability to add any new numbers of Thedds collections, there is no need to modify the code anymore. So more flexibility and less "paperwork time" in PR reviews. This stack will hopefully be deployed by many different organisations with very different needs, we should care for flexibility to help meet their needs. |
In a way, there is nothing more that needs to be done than adding a custom The issue with combining everything under a common location/directory/component is that it can limit configuration mounts of the server. For now, only a simple Nginx birdhouse-deploy/birdhouse/config/proxy/nginx.conf.template Lines 27 to 34 in cc8dc1f Another reason that I named the component |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2309/Result : success BIRDHOUSE_DEPLOY_BRANCH : stac-data-proxy DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1439/NOTEBOOK TEST RESULTS |
|
I think that it is misleading to name this component "stac" since it really doesn't have anything to do with STAC (it just serves files that could be referenced by a STAC catalog but doesn't need to be at all). I think that we should use the
I agree with this. However, having to make a new optional component every single time we need a new mountpoint is going to be very confusing. It would be better to make this component configurable so that it can support multiple mountpoints with different nginx location definitions if needed. |
|
Agreed |
Component wise, this is true for now with minimal nginx config. However, I believe STAC has an important role being an eventual default component that could justify it. As shown by the component combination table here, the multiple flavors in which STAC can be configured is already somewhat complex depending on use cases the node wants to support. I think we can help new node managers configure their server more efficiently and quickly by providing explicit references to STAC rather than more extra variables to configure.
Wouldn't that mean that each of those mount point and nginx configuration will require to define a new component anyway to apply relevant Just to make sure my point is understood properly. The idea of having a separate nginx config and component for STAC data specifically is that any relevant optimization for accessing STAC data could be gradually defined for that path. Other optimizations or redirections could be applied as we investigate further what works best for STAC and how users interact with it. This is not the same as the case for THREDDS where all configurable datasets go through THREDDS. Since I think location /more-custom-data/ {
alias /data/more-custom-data/;
}Therefore, a node admin could already very easily add more data access points as needed without the need of an additional component. They are also free to choose if the (eventually) predefined optimization that would be provided by |
|
Ok I'm fine with that in principle. I'll have a look in detail and leave a review shortly |
There was a problem hiding this comment.
I just have a few comments about the documentation and wording of this.
Other than that and the change suggested in this other comment (https://github.com/bird-house/birdhouse-deploy/pull/403/files#r1407965025) I think this looks good
birdhouse/optional-components/stac-data-proxy/config/magpie/permissions.cfg.template
Outdated
Show resolved
Hide resolved
birdhouse/optional-components/stac-data-proxy/config/magpie/permissions.cfg.template
Outdated
Show resolved
Hide resolved
...house/optional-components/stac-public-access/config/stac-data-proxy/permissions.cfg.template
Show resolved
Hide resolved
@fmigneault I wasn't aware the following up PRs that could be STAC-specific. In this case, I agree to make this new component STAC-specific. Also I agree it is fairly easy to add new Nginx config to handle new URL so the value to have a generic component for static file is less "valuable". |
dchandan
approved these changes
Nov 29, 2023
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2316/Result : failure BIRDHOUSE_DEPLOY_BRANCH : stac-data-proxy DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1445/NOTEBOOK TEST RESULTS |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2317/Result : failure BIRDHOUSE_DEPLOY_BRANCH : stac-data-proxy DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-133.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1446/NOTEBOOK TEST RESULTS |
mishaschwartz
approved these changes
Nov 30, 2023
|
run tests |
12 tasks
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2323/Result : success BIRDHOUSE_DEPLOY_BRANCH : stac-data-proxy DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-133.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1451/NOTEBOOK TEST RESULTS |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2325/Result : failure BIRDHOUSE_DEPLOY_BRANCH : stac-data-proxy DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-133.rdext.crim.ca Infrastructure deployment failed. Instance has not been destroyed. @matprov |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2324/Result : failure BIRDHOUSE_DEPLOY_BRANCH : stac-data-proxy DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-154.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1452/NOTEBOOK TEST RESULTS |
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
Provide a way to host local data that STAC API can refer to for use/download.
Currently, any STAC Asset that is referenced within responses by STAC-API Collections/Items must either be already hosted by another service of the stack (eg: CMIP6 netCDF in THREDDS), or point at some other external resource not on the server.
Instead of having a custom config and mount point for each node, this optional component defines a standard way to define it.
Changes
Non-breaking changes
optional-components/stac-data-proxy: add a new feature to allow hosting of local STAC assets.The new component defines variables
STAC_DATA_PROXY_DIR_PATH(default${DATA_PERSIST_ROOT}/stac-data) andSTAC_DATA_PROXY_URL_PATH(default/data/stac) that are aliased (mapped) undernginxto provide a URLwhere locally hosted STAC assets can be downloaded from. This allows a server node to be a proper data provider,
where its STAC-API can return Catalog, Collection and Item definitions that points at these local assets available
through the
STAC_DATA_PROXY_URL_PATHendpoint.When enabled, this component can be combined with
optional-components/secure-data-proxyto allow per-resourceaccess control of the contents under
STAC_DATA_PROXY_DIR_PATHby setting relevant Magpie permissions under servicesecure-data-proxyfor children resources that correspond toSTAC_DATA_PROXY_URL_PATH. Otherwise, the path andall of its contents are publicly available, in the same fashion that WPS outputs are managed without
optional-components/secure-data-proxy.More details are provided in https://github.com/bird-house/birdhouse-deploy/blob/stac-data-proxy/birdhouse/optional-components/README.rst#provide-a-proxy-for-local-stac-asset-hosting
Breaking changes
Related Issue / Discussion
STAC metadata generated from above notebook (see subset for example), will be able to use a location such as
https://${PAVICS_FQDN_PUBLIC}${STAC_DATA_PROXY_URL_PATH}/EuroSAT/...instead of the temporary raw-GitHub content URLs. The STAC populator (withDirectoryLoadingimplementation), will be able to push the STAC Collection/Items toward that instances. The STAC Assets that they refer to will be placed under${STAC_DATA_PROXY_DIR_PATH}/EuroSATto make them accessible externally.