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

Add environment variable to disable Transaction extension in provided pgstac app #495

Merged

Conversation

jtherrmann
Copy link
Contributor

See #492

@geospatial-jeff
Copy link
Collaborator

@jtherrmann thinking of how to make this more applicable to the other extensions. How about an environment variable ENABLED_EXTENSIONS which accepts a comma-delimited string (ex. transactions,sort,query) determining what extensions are added to the application.

If it is not present all extensions would be enabled.

@jtherrmann
Copy link
Contributor Author

jtherrmann commented Nov 16, 2022

@geospatial-jeff Is there a use-case for disabling other extensions? The Transaction extension is the only one that I would want to disable, for security reasons, but I can't think of a reason for disabling any of the others.

Edit: In any case, take a look at the most recent commit and let me know if that's along the lines of what you were suggesting.

@gadomski
Copy link
Member

CI is breaking because we need to ceil the fastapi version (e.g. cb76e2e). @geospatial-jeff if you could approve #487, we can merge that to pick up the fix.

@gadomski
Copy link
Member

Thanks @geospatial-jeff! @jtherrmann if you update with main and re-push hopefully things will fix.

@geospatial-jeff
Copy link
Collaborator

Is there a use-case for disabling other extensions? The Transaction extension is the only one that I would want to disable, for security reasons, but I can't think of a reason for disabling any of the others.

I agree that transactions is probably the one most people would want to remove. The reason to allow users to disable other is that extensions in the STAC API spec are optional (add-ons), and therefore should be allowed to be disabled even if that functionality is not used by most people.

Copy link
Collaborator

@geospatial-jeff geospatial-jeff left a comment

Choose a reason for hiding this comment

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

This LGTM! Thanks very much @jtherrmann 🙏

@jtherrmann
Copy link
Contributor Author

Thanks @geospatial-jeff! When can we expect a release so that we can use this functionality (without installing from our fork)?

@geospatial-jeff
Copy link
Collaborator

I can cut the next release this weekend, there are a few other outstanding PRs I want to get merged.

@jtherrmann
Copy link
Contributor Author

@geospatial-jeff Great, thanks!

@geospatial-jeff
Copy link
Collaborator

Sorry for the delay, I got a nasty case of the flu and have been out the past week. I will cut a new release today @jtherrmann . Thanks for your patience!

@geospatial-jeff geospatial-jeff merged commit b00370e into stac-utils:master Nov 25, 2022
@geospatial-jeff geospatial-jeff mentioned this pull request Nov 25, 2022
4 tasks
@jtherrmann
Copy link
Contributor Author

@geospatial-jeff No worries, hope you're feeling better! Thanks!!

@jtherrmann jtherrmann deleted the optional-disable-transactions branch November 28, 2022 18:47
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.

None yet

3 participants