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

Configure the landing page id, description, etc. via env vars #639

Merged
merged 4 commits into from
Apr 11, 2024

Conversation

m-mohr
Copy link
Contributor

@m-mohr m-mohr commented Mar 11, 2024

Related Issue(s):

  • None?

Description:

Allow an easy way to configure the landing page id, description, title and version via env variables.
Maybe there's another way to do it properly, but it was not documented and on Gitter no one seemed to know a way wither.
So trying to enable a way to set the information via environment variable.

For the landing page, you can set the API title, description and version using environment variables.

  • STAC_FASTAPI_VERSION (string) is the version number of your API instance (this is not the STAC version).
  • STAC FASTAPI_TITLE (string) should be a self-explanatory title for your API.
  • STAC FASTAPI_DESCRIPTION (string) should be a good description for your API. It can contain CommonMark.
  • STAC_FASTAPI_ID (string) is a unique identifier for your API.

PR Checklist:

  • pre-commit hooks pass locally
  • Tests pass (run make test)
  • Documentation has been updated to reflect changes, if applicable, and docs build successfully (run make docs)
  • Changes are added to the CHANGELOG.

@m-mohr m-mohr added the enhancement New feature or request label Mar 11, 2024
@jamesfisher-geo
Copy link
Contributor

jamesfisher-geo commented Mar 11, 2024

Should this be defined here, or in the backend implementations when they call the StacApi class?

See this PR

@m-mohr
Copy link
Contributor Author

m-mohr commented Mar 11, 2024

I believe it's fine here as it's just the fallback for when it's not passed from the implementations. It would fail if the implementatons pass a default value, too.

Also, I believe that this is something that is not backend specific and as such should in general be in the common implementation, otherwise the behavior is different depending on tha backend. That is not logical to me.

If we go forward with stac-utils/stac-fastapi-elasticsearch-opensearch#207, we should align the variable names @jamesfisher-gis

@jamesfisher-geo
Copy link
Contributor

jamesfisher-geo commented Mar 11, 2024

I believe it's fine here as it's just the fallback for when it's not passed from the implementations. It would fail if the implementatons pass a default value, too.

Actually, none of the implementations include a title or description when calling the StacApi class.

This would fail if any title or description string is passed to the StacApi class. Rather, it would quietly use either the environment variable or default value and ignore the user-defined value.

Also, I believe that this is something that is not backend specific and as such should in general be in the common implementation, otherwise the behavior is different depending on tha backend. That is not logical to me.

Agreed. But from a code readability perspective I think it is better to have the title and description defined when calling the StacApi class in the implementation and leave the StacApi class to impose the baseline default ("stac-fastapi").

If we go forward with stac-utils/stac-fastapi-elasticsearch-opensearch#207, we should align the variable names @jamesfisher-gis

Yup. Regardless I changed the env vars here to match yours.

@m-mohr
Copy link
Contributor Author

m-mohr commented Mar 12, 2024

This would fail if any title or description string is passed to the StacApi class. Rather, it would quietly use either the environment variable or default value and ignore the user-defined value.

Why is that? I guess then I don't understand how attrs works.

@jamesfisher-geo
Copy link
Contributor

This would fail if any title or description string is passed to the StacApi class. Rather, it would quietly use either the environment variable or default value and ignore the user-defined value.

Why is that? I guess then I don't understand how attrs works.

No, you are right. I forgot that the get env was in the default argument.

@jonhealy1
Copy link
Collaborator

We are actually already using this in stac-fastapi-elasticsearch-opensearch. stac-utils/stac-fastapi-elasticsearch-opensearch#207

@jonhealy1
Copy link
Collaborator

Could we add this to v2.5.0 and then leading up to v3.0.0 try to reconcile this with #499?

* use pydantic settings

* rename stac_fastapi_id to stac_fastapi_landing_id
@vincentsarago vincentsarago self-requested a review April 11, 2024 14:50
@vincentsarago vincentsarago merged commit 96de0e2 into main Apr 11, 2024
7 checks passed
@vincentsarago vincentsarago deleted the landing-page-config-via-env branch April 11, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants