kbgg
commented
3 years ago We're getting the same error when adding our Radiant MLHub collections which all extend the scientific extension
Open csaybar opened 3 years ago
kbgg
commented
3 years ago We're getting the same error when adding our Radiant MLHub collections which all extend the scientific extension
lossyrob
commented
3 years ago I've run into a similar problem - this is due to a current limitation of stac-fastapi on handling custom models. The limitation is described here: https://github.com/stac-utils/stac-fastapi/issues/58. The issue you're seeing is the top-level "sci:citiation" property doesn't map to any stac-pydantic field. Even if you were able to use a subclassed stac-pydantic model that did add that field, the SQLAlchemy backend wouldn't support it because there's no column for it in the database model. This is a tricky issue that had me stuck on a fork of arturo-stac-api, in which I added a database change to account for this in this commit.
The upcoming pgstac backend will not have the problem on the database side, as it will use JSONB for all the Collection (and also Item) properties and won't need additional columns for custom data fields. However, the stac-pydantic model subclassing issue will still remain.
I think a solution to this would be to add a setting to ApiSettings that lets the users of stac-fastapi to provide subclasses of stac-pydantic models that they want to use in place of the original models. This would let custom models to be set when declaring the API, and allow custom fields to be put into place. There you could pass in a custom Collection subclass with the that looks similar to the custom Collection used in the commit linked to above.
The sore spot with that solution is that, if you encounter another extension, the custom model will have to be updated to account for that. I don't believe there's a catch-all in stac-pydantic to deal with arbitrary extensions (or to just allow properties that it doesn't recognize). This might be a reason to consider relaxing the validation constrictions on the pydantic models, or even to move to PySTAC as the internal STAC model (though we'd lose some of the FastAPI <-> Pydantic nicities, which tbh I've found more confusing/in the way than beneficial, at least besides the auto OpenAPI schema definitions).
geospatial-jeff
commented
3 years ago I think a solution to this would be to add a setting to
ApiSettingsthat lets the users of stac-fastapi to provide subclasses of stac-pydantic models that they want to use in place of the original models.
Agreed, ApiSettings is the "bucket of state" for all the things that don't quite fit elsewhere, and I think its fine to continue this pattern until it becomes unsustainable and needs to be rethought. The one hiccup I see here is it might make testing harder, as we need an instance of ApiSettings to test the backend clients which seems to be a violation of separation of concerns, but this might be unavoidable.
I don't believe there's a catch-all in stac-pydantic to deal with arbitrary extensions (or to just allow properties that it doesn't recognize)
You can set extra = 'allow' in the model config (https://pydantic-docs.helpmanual.io/usage/model_config/) to allow arbitrary properties which aren't defined in the model itself.
This might be a reason to consider relaxing the validation constrictions on the pydantic models, or even to move to PySTAC as the internal STAC model
We've done this for the GET /search and POST /search endpoints because the fields extension can exclude fields that are required by the underlying pydantic models. I thought it worked out nicely, might be worth trying this with the other endpoints to allow for more flexibility with the model definitions.
moradology
commented
3 years ago After some discussion with @geospatial-jeff and @lossyrob, I've spiked out a small prototype of what would be needed with the current SqlAlchemy backend to get custom models working. There are definitely some issues that need to be ironed out but which probably require inputs from people more involved in library design than me; I've highlighted the two most pressing concerns in the body of the draft (#134)
Hi again!
When I try to extent the collection fields, for instance, using the scientific extension (example)
sci:citationI got the next error.collection.json
error
This is really curious because when I try to extent with some other fields like keyword no errors appears.
collection.json
error
NO ERROR
I tried to modify the files '/stac_api/models/schemas.py' and
/stac_fastapi/sqlalchemy/models/database.pybut with no success any help will be appreciated!PS: I tried the same in versions 1.1.0 and 1.0.0 and this problem persist.