Skip to content

Conversation

@alessandro-nori
Copy link
Contributor

@alessandro-nori alessandro-nori commented May 7, 2025

Rationale for this change

list_namespaces should raise a NoSuchNamespaceException instead of a generic RestException in case of a 404 response.

https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L279-L287

Also this is in line with the Java implementation of NamespaceErrorHandler used by [listNamespaces()](https://github.com/apache/iceberg/blob/9fb80b7167e58a2daef80f4a1a09f223b870c030/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java#L584)

Are these changes tested?

I added a unit test for it

Are there any user-facing changes?

@alessandro-nori alessandro-nori force-pushed the fix_no_ns_exception_rest branch from 2f56a35 to 461a9d7 Compare May 7, 2025 07:58
Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@kevinjqliu kevinjqliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! LGTM

@kevinjqliu kevinjqliu merged commit c84017d into apache:main May 7, 2025
10 checks passed
gabeiglio pushed a commit to Netflix/iceberg-python that referenced this pull request Aug 13, 2025
…1977)

<!--
Thanks for opening a pull request!
-->

<!-- In the case this PR will resolve an issue, please replace
${GITHUB_ISSUE_ID} below with the actual Github issue id. -->
<!-- Closes #${GITHUB_ISSUE_ID} -->

# Rationale for this change

`list_namespaces` should raise a `NoSuchNamespaceException` instead of a
generic `RestException` in case of a 404 response.


https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L279-L287

Also this is in line with the Java implementation of
[NamespaceErrorHandler](https://github.com/apache/iceberg/blob/9fb80b7167e58a2daef80f4a1a09f223b870c030/core/src/main/java/org/apache/iceberg/rest/ErrorHandlers.java#L182)
used by
`[listNamespaces()](https://github.com/apache/iceberg/blob/9fb80b7167e58a2daef80f4a1a09f223b870c030/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java#L584)`

# Are these changes tested?

I added a unit test for it

# Are there any user-facing changes?

<!-- In the case of user-facing changes, please add the changelog label.
-->
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.

3 participants