Skip to content
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

feat: Add topo-imagery version information to STAC TDE-1265 #1080

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

l0b0
Copy link
Contributor

@l0b0 l0b0 commented Sep 19, 2024

Motivation

As a data maintainer, geospatial data engineer or data consumer, I would like to reference the technical configuration used to create the TIFFs in our ODR datasets in order to be able to recreate the TIFFs or carry out troubleshooting.

Modifications

Add processing:software, processing:version, and processing:datetime item properties.

Verification

pytest

@l0b0 l0b0 requested a review from a team as a code owner September 19, 2024 00:09
@l0b0 l0b0 force-pushed the feat/add-technical-config-fields-to-stac branch from 5d5b704 to 542bf58 Compare September 19, 2024 00:59
@l0b0 l0b0 force-pushed the feat/add-technical-config-fields-to-stac branch from 542bf58 to cd888fc Compare September 19, 2024 01:00
@amfage amfage removed the request for review from MDavidson17 September 19, 2024 01:43
@@ -27,7 +27,7 @@ The scripts have been implemented to be run inside the Docker container only. Th
- Build the `Docker` image:

```bash
docker build . -t topo-imagery
docker build --build-arg=GIT_HASH=dev --build-arg=GIT_VERSION=dev --tag=topo-imagery .
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to make the local build commands more complicated? what value does adding "dev"/"dev" add here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was some discussion about the best way to check that these exist when running in CI, open to suggestions! I think you are right we should change it as I can foresee they will end up commented out in the Dockerfile which would be a bad sign.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the first command in the README.md it is likely what people will run to first build this system, is making this command much more complicated to add "dev" "dev" seem excessive?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for making these mandatory was to ensure that we don't forget to add them, which would only be detected at runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants