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

PUT "/collections/{collection_id}" throws "update_collection() missing 1 required positional argument: 'collection'" #665

Closed
avbentem opened this issue Apr 19, 2024 · 3 comments

Comments

@avbentem
Copy link
Contributor

#630 fixed the update collection REST endpoint in the "transactions" extension, to properly include the collection's id in /collections/{collection_id}. Nice.

However, when the endpoint is registered it still only expects a Collection in the JSON payload, without defining the new URL segment. This makes the first update_collection argument collection_id being passed the full JSON payload, and makes collection being empty.

This throws update_collection() missing 1 required positional argument: 'collection'.

I'll create a PR with a fix.

@avbentem avbentem mentioned this issue Apr 19, 2024
4 tasks
@jonhealy1
Copy link
Collaborator

Hi @avbentem. Thanks for this. What stac-fastapi backend are you using?

@avbentem
Copy link
Contributor Author

Hi @jonhealy1 sorry for only seeing your question today. I'm not actually using any backend yet. (I'll likely use a bespoke one, combing Google cloud storage with PostGIS.) But I ran into this bug when setting up a basic project structure, that basically only implements the abstract methods to print the requests. (Kind of like in the tests I added.)

Wondering why you're asking. Do you think that some of the stac-utils provided backends (like stac-fastapi-pgstac or stac-fastapi-sqlalchemy) would not suffer from this bug? If that would be the case then I'd guess that those backends are not using the abstract classes that this very stac-fastapi provides?

@jonhealy1
Copy link
Collaborator

jonhealy1 commented Apr 23, 2024

Ok thanks for the clarification. stac-fastapi-pgstac and stac-fastapi-elasticsearch-opensearch seem to be fine. You may be right - something to look into.

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

No branches or pull requests

2 participants