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 request body types for Transaction Extension spec and client #574

Merged

Conversation

mmcfarland
Copy link
Contributor

@mmcfarland mmcfarland commented May 11, 2023

Description:

A POST against an items endpoint should accept an Item or ItemCollection

https://github.com/radiantearth/stac-api-spec/blob/master/ogcapi-features/extensions/transaction/README.md#methods

Added literal values to the stac_types type attribute in order to allow fastapi a discriminator value for parsing the request body to the correct TypedDict. Without this change, ItemCollections were parsed as Items and lost the features attribute.

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.

A POST against an items endpoint should accept an Item or ItemCollection

https://github.com/radiantearth/stac-api-spec/blob/master/ogcapi-features/extensions/transaction/README.md#methods

Add literal values to the stac_types `type` attribute in order to allow fastapi a
discriminator value for parsing the request body to the correct TypedDict.
Copy link
Member

@gadomski gadomski left a comment

Choose a reason for hiding this comment

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

@mmcfarland LGTM. I added a pretty basic smoke test, I don't think it actually exercises much (maybe some of the typing changes) but it at least demonstrates what we're trying to do? Do you mind taking a look at the test and seeing if it makes sense and if you think it has any value?

It doesn't exercise much, but at least it demonstrates what we're trying to do.
@gadomski gadomski force-pushed the mjm/transaction-create-itemcollection branch from e33a3af to d7e2c65 Compare May 12, 2023 13:06
@mmcfarland
Copy link
Contributor Author

Thanks @gadomski I think it definitely adds value. I ran the test against main and got the expected failures. Thanks for the assist!

@gadomski gadomski merged commit 74a7909 into stac-utils:main May 12, 2023
5 checks passed
@gadomski gadomski added this to the 2.4.7 milestone May 12, 2023
@mmcfarland mmcfarland deleted the mjm/transaction-create-itemcollection branch May 15, 2023 15:48
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

2 participants