Nsr/580/missing collection error#1029
Conversation
theodorehreuter
left a comment
theodorehreuter
left a comment
There was a problem hiding this comment.
one minor nitpick regarding error message composition
| export function isItem(record) { | ||
| return record && record.type === 'Feature' | ||
| if (record && record.type === 'Feature') { | ||
| if ('collection' in record) { |
There was a problem hiding this comment.
I'm assuming we can't do any model validation and are just directly checking for the key since this is just vanilla JS?
There was a problem hiding this comment.
Pretty much, yeah. There is a library (that is used in other places in stac-server) called zod that is pretty similar to pydantic in the JS world, but there isn't the level of integration with the web framework in this case as in the FastAPI world. I definitely thought about adding full on payload model validation, but that would be a significant addition and it felt like a lot for a small bugfix. Definitely open to creating a ticket suggesting adding it...
There was a problem hiding this comment.
I feel like all the model stuff should probably come with the typescript change over. I know that's gonna be a substantial move so not sure where that fits on the roadmap.
There was a problem hiding this comment.
Yeah, that's a good point, the model/typing integration could work really well.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
* require Items to have collection * Update invalid quick links in README * update changelog * update error message * fix audit errors
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
2 participants
Related Issue(s):
collectionis missing from ingesting Item, a low-level OpenSearch client error occurs rather than a clear invalid response #580Proposed Changes:
isItemthrows an error if thecollectionfield is not set. This logs a more helpful message than before, when this caused an OpenSearch error.PR Checklist: