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 for stac-fastapi v3.0.0a release #234

Merged
merged 36 commits into from
May 10, 2024
Merged

Update for stac-fastapi v3.0.0a release #234

merged 36 commits into from
May 10, 2024

Conversation

jonhealy1
Copy link
Collaborator

@jonhealy1 jonhealy1 commented Apr 27, 2024

Related Issue(s):

Description:

Update stac-fastapi parent libraries to v3.0.0a. There are quite a few changes made in this pr.

PR Checklist:

  • Code is formatted and linted (run pre-commit run --all-files)
  • Tests pass (run make test)
  • Documentation has been updated to reflect changes, if applicable
  • Changes are added to the changelog

@jonhealy1
Copy link
Collaborator Author

All of the filters tests/ functionality is broken

@jonhealy1
Copy link
Collaborator Author

This is working now with stac-fastapi v3.0.0a #238

@jonhealy1 jonhealy1 marked this pull request as ready for review May 7, 2024 14:20
@jonhealy1
Copy link
Collaborator Author

Some tests are passing locally in docker but failing here

@jonhealy1
Copy link
Collaborator Author

I have no idea why this started failing after I merged the new changes from main - I will have to figure it our tomorrow ...

@jonhealy1
Copy link
Collaborator Author

just this I guess in execute_search max_result_window = stac_fastapi.types.search.Limit.le something changed

@jonhealy1 jonhealy1 changed the title Upcoming stac-fastapi v3.0.0 release Update for stac-fastapi v3.0.0a release May 9, 2024
@pedro-cf
Copy link
Collaborator

pedro-cf commented May 9, 2024

Can add this issue as related issue:

@jonhealy1
Copy link
Collaborator Author

@pedro-cf Will this fix that issue?

@pedro-cf
Copy link
Collaborator

pedro-cf commented May 9, 2024

@pedro-cf Will this fix that issue?

yes like I meantioned in the issue:

Additional context

stac_pydantic.Item(**feat).json(**filter_kwargs, exclude_unset=True)

Issue seems to be the version of stac_pydantic==2.0.*, I believe this will be fixed if upgraded to stac_pydantic==3.0.0 with stac-fastapi.api==3.0.0

@jonhealy1
Copy link
Collaborator Author

Maybe we should write a test for this? Would you like to be a maintainer of this repo? I sent you an invite just in case.

@jonhealy1 jonhealy1 requested a review from pedro-cf May 9, 2024 12:00
@jonhealy1 jonhealy1 mentioned this pull request May 9, 2024
@pedro-cf
Copy link
Collaborator

pedro-cf commented May 9, 2024

@jonhealy1 added test_item_custom_links

@jonhealy1
Copy link
Collaborator Author

linting error

@jonhealy1
Copy link
Collaborator Author

test looks great

@jonhealy1
Copy link
Collaborator Author

@pedro-cf extra inputs are not permitted in Item - something may need to change in stac-pydantic?

@pedro-cf
Copy link
Collaborator

pedro-cf commented May 9, 2024

@pedro-cf extra inputs are not permitted in Item - something may need to change in stac-pydantic?

yes that can be changed: https://docs.pydantic.dev/2.7/api/config/#pydantic.config.ConfigDict.extra

but should it ?

Not sure how to proceeed here, maybe this should not be allowed? kinda loses the point of this plugin though: https://github.com/stac-extensions/web-map-links

@pedro-cf
Copy link
Collaborator

pedro-cf commented May 9, 2024

perhaps it's best to revert this test

@jonhealy1
Copy link
Collaborator Author

jonhealy1 commented May 10, 2024

Let's revert the test for now and talk about this in #247. I think this is an issue with stac-pydantic according to this - https://schemas.stacspec.org/v1.0.0/item-spec/json-schema/item.json - it should work

@jonhealy1 jonhealy1 merged commit a1d076a into main May 10, 2024
5 checks passed
@jonhealy1 jonhealy1 deleted the stac-fastapi-v3.0 branch May 10, 2024 10:42
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