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

update stac-version #762

Merged
merged 1 commit into from
Oct 15, 2024
Merged

update stac-version #762

merged 1 commit into from
Oct 15, 2024

Conversation

vincentsarago
Copy link
Member

ref #761

@m-mohr
Copy link
Contributor

m-mohr commented Oct 14, 2024

Thanks, code changes look good to me.

I'm just wondering whether there should be a test for it?
So if the env variable is set, whether the response actually changes?
Also, is this documented somewhere?

I'm asking because I heard from a colleague that he was actually setting STAC_API_VERSION in env but it had no effect. (Or is it meant to be stac_pydantic.version.STAC_API_VERSION before?) I didn't check myself, just wondering.

@vincentsarago
Copy link
Member Author

@m-mohr I don't think there is any env variable that can change what is set in https://github.com/stac-utils/stac-fastapi/blob/main/stac_fastapi/api/stac_fastapi/api/app.py#L96
https://github.com/stac-utils/stac-fastapi/blob/main/stac_fastapi/types/stac_fastapi/types/core.py#L287

Worth noting that I don't think this should be configurable because it all comes down to what version for the spec is supported by stac-pydantic

I still think that it might be good to indicate which stac and stac-api version the application provide (from stac-pydantic)

@m-mohr
Copy link
Contributor

m-mohr commented Oct 14, 2024

Oh dang, I thought that's a env variable similar to STAC_API_TITLE etc.
Seems like a misunderstanding then. In 1.0 and 1.1 the changes are not that big so I guess just updating the STAC version number to 1.1.0 for some instances could make sense but also for others to stay with 1.0.0.

So +1 to: "I still think that it might be good to indicate which stac and stac-api version the application provide (from stac-pydantic)" - could that be simply an env variable?

@vincentsarago
Copy link
Member Author

vincentsarago commented Oct 15, 2024

@m-mohr

So +1 to: "I still think that it might be good to indicate which stac and stac-api version the application provide (from stac-pydantic)" - could that be simply an env variable?

So there are 2 things:

  • STAC VERSION: should indicate the STAC version specification returned by the endpoints, those should match what stac-pydantic has because we use it (optional) for validation, so I can't think about a situation where the STAC Version number would be different from the one from stac-pydantic
  • STAC API VERSION: this is almost the same (models used to validate response) but add also the spec for what endpoints are available. I'm having hard time to think why/how user would have a stac-fastapi based application which don't follow the version forwarded from stac-pydantic

could that be simply an env variable?

short answer: I don't think so

@jonhealy1
Copy link
Collaborator

jonhealy1 commented Oct 15, 2024

A stac api should work with any stac version, even a catalog that may be a mix of v1 and v1.1?

@m-mohr
Copy link
Contributor

m-mohr commented Oct 15, 2024

Right, so we need stac-pydantic to support both STAC 1.0.0 and STAC v1.1.0 soon.

@vincentsarago
Copy link
Member Author

@jonhealy1

A stac api should work with any stac version, even a catalog that may be a mix of v1 and v1.1?

🤷 depends if the 1.1 is backward compatible with 1.0

@m-mohr
Copy link
Contributor

m-mohr commented Oct 15, 2024

It is, I'd say... otherwise it should've been a 2.0.

@vincentsarago
Copy link
Member Author

IMO: if someone wants to change the STAC VERSION, it means they have a custom version of stac-fastapi, which also means they don't really need to control this with an Env variable

@vincentsarago vincentsarago merged commit 8235d91 into main Oct 15, 2024
7 checks passed
@vincentsarago vincentsarago deleted the patch/stac-api-to-stac-version branch October 15, 2024 11:20
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.

4 participants