SPEC: Add 404 response for /v1/config endpoint#15746
SPEC: Add 404 response for /v1/config endpoint#15746oguzhanunlu wants to merge 1 commit intoapache:mainfrom
Conversation
kevinjqliu
left a comment
There was a problem hiding this comment.
Thanks for re-opening this, the last one was lost in my email..
Throwing 404 here makes sense. Lets get this PR cleaned up and have just the rest spec changes.
Could you also start a thread on the iceberg devlist?
I've found a reference of past spec change discussion that you can use as a reference, https://lists.apache.org/thread/01gb9rygdd1gqks7lnl1o6440qocnh9m
You can just include your original PR's justification in the email.
d31b64e to
f26e0ee
Compare
|
Thanks @kevinjqliu , I cleared the PR to have the spec change only and started a devlist thread at https://lists.apache.org/thread/07t5rfcxn3bbbpp75fkr2h1mwdp2o4mn with justification. |
Add NoSuchWarehouseResponse to document HTTP 404 as a valid response for the /v1/config endpoint when a requested warehouse does not exist.
f26e0ee to
3960939
Compare
| 403: | ||
| $ref: '#/components/responses/ForbiddenResponse' | ||
| 404: | ||
| $ref: '#/components/responses/NoSuchWarehouseResponse' |
There was a problem hiding this comment.
both PR and devlist thread mentions NotFoundException, should we change it to that?
There was a problem hiding this comment.
I'm personally a fan of not adding new response types.
We expect tools/libraries to be checking against the HTTP status codes when response parsing. But, if they're checking against the type, then those tools need to be updated to understand this new WarehouseNotFound exception.
Summary
This PR proposes adding HTTP 404 as a documented response for the
/v1/configendpoint in the REST Catalog OpenAPI specification.Rationale
The
/v1/configendpoint allows an optional query parameter for a warehouse identifier, e.g./v1/config?warehouse=mywarehouse. But the openapi spec does not specify what should happen if the requested warehouse does not exist.We noticed that Snowflake Open Catalog already returns a 404 for non-existent warehouses:
{ "error": { "message": "Unable to find warehouse NONEXISTENT_WAREHOUSE_12345", "type": "NotFoundException", "code": 404 } }This PR therefore formalizes what Snowflake Open Catalog is already doing in production. It seems sensible to formalize the 404 response code, because this is consistent with other Iceberg REST endpoints which allow a 404 response code for missing resources (tables, namespaces, views).
Our use case
At my company we triage Iceberg exceptions according to their type. For example, an exception for a missing resource might get triaged to the team responsible for providing that resource.
Currently, the Iceberg core library yields the generic
RESTExceptionif a warehouse does not exist. It is difficult to automatically triage the genericRESTException. It would help us if Iceberg could yield theNoSuchWarehouseExceptioninstead, so our application code can more confidently triage this runtime issue.We expect other Iceberg users have similar operational workflows where distinguishing warehouse configuration errors from other failures would improve their incident response.
Context
Split from #14965 per @danielcweeks's suggestion to discuss and vote on the spec change separately from the implementation.
See the dev list thread: https://lists.apache.org/thread/07t5rfcxn3bbbpp75fkr2h1mwdp2o4mn