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

check server stack version #79

Merged
merged 4 commits into from
Mar 4, 2025
Merged

Conversation

mishaschwartz
Copy link
Contributor

@mishaschwartz mishaschwartz commented Feb 25, 2025

Adds checks to ensure that the version of the STAC spec that the server is running matches the one used by pystac before uploading any data.

Allows the user to specify the version used by the STAC catalog that is being uploaded to. Users can specify this either with a command line option or with an environment variable PYSTAC_STAC_VERSION_OVERRIDE

See initial discussion here: #73 (comment)

pyproject.toml Outdated
Comment on lines 173 to 177
stac_spec_latest = [
"pystac~=1.12.1"
]
stac_spec_10 = [
"pystac~=1.11.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this solves the issue.
It forces a lock in to older versions that might not receive appropriate bug fixes, whereas the STAC 1.0/1.1 are supposed to be backward-compatible.

The idea is to allow the creation of STAC 1.0 collections/items in case the remote server receiving them does not yet support 1.1, which could cause invalid parsing.

It seems the solution to invoke a pystac.set_stac_version with the desired version based on
stac-utils/pystac#1375 (comment)
And pystac>=1.12.1 would always be employed.

Ideally, the pystac.set_stac_version would be invoked with the applicable stac_version returned by the addressed catalog by URL, and the populator could optionally offer an override argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fmigneault

Ok it sounds like you read this comment (#73 (comment)) after reviewing this here.

It forces a lock in to older versions that might not receive appropriate bug fixes, whereas the STAC 1.0/1.1 are supposed to be backward-compatible.

Yes, this is not ideal but we're limited by decisions made by the pystac developers.

The idea is to allow the creation of STAC 1.0 collections/items in case the remote server receiving them does not yet support 1.1, which could cause invalid parsing.

It would not be invalid. See stac-utils/pystac#1375 (comment)

Ideally, the pystac.set_stac_version would be invoked with the applicable stac_version returned by the addressed catalog by URL, and the populator could optionally offer an override argument.

If you believe that is sufficient then let's implement that

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since it wouldn't be invalid to have the extra bands/eo:bands/raster:bands whether using 1.0 or 1.1, I think using pystac.set_stac_version would be the right approach.

If a server advertises that it supports only 1.0, it will simply ignore the extra 1.1 parameters, but won't start complaining that we sent it items marked as 1.1. That version would raise an error due to https://github.com/radiantearth/stac-spec/blob/ec002bb93dbfa47976822def8f11b2861775b662/catalog-spec/json-schema/catalog.json#L27-L31 if the server only handled 1.0.

By default, it seems reasonable to use 1.1.
The pystac.set_stac_version should be only an override in case of problem.
Either way, the latest pystac would be used.

Base automatically changed from fix-tests-variable-stac-version to master February 26, 2025 15:16
Copy link
Collaborator

@fmigneault fmigneault left a comment

Choose a reason for hiding this comment

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

Approving with temporary workaround to resolve STAC 1.1, conditionally to #79 post-check before bump release.

@fmigneault fmigneault merged commit 11a1168 into master Mar 4, 2025
11 checks passed
@mishaschwartz mishaschwartz deleted the check-server-stack-version branch March 4, 2025 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants