Skip to content

Conversation

@jayceslesar
Copy link
Contributor

When adding common catalog integration tests in pyiceberg, we noticed that the rest catalog test, which tests against the rest fixture was failing to throw a NoSuchNamespaceError when attempting to create a table in a namespace that does not exist. @kevinjqliu spotted the issue and we are able to resolve downstream by setting CATALOG_JDBC_STRICT__MODE=true in the docker-compose-integration.yml, but I think it makes sense to set that variable here to ensure we have consistent behavior wherever this fixture is used.

Tests are now passing downstream with the environment variable set, and the rest catalog is behaving in line with other catalog implementations in the integration tests, which I believe is proof enough that it is safe to make this change here.

@jayceslesar jayceslesar changed the title fix: ensure strict mode on jdbc catalog for rest fixture REST-Fixture: Ensure strict mode on jdbc catalog for rest fixture Jul 20, 2025
Co-authored-by: Fokko Driesprong <fokko@apache.org>
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.

LGTM i verified this locally when testing apache/iceberg-python#2090

With this change, tables can no longer be created without first creating its namespace in the iceberg-rest-fixture REST server. This aligns with the general expectation when creating a table in a catalog.

@kevinjqliu
Copy link
Contributor

kevinjqliu commented Jul 20, 2025

With this change, tables can no longer be created without first creating its namespace in the iceberg-rest-fixture REST server. This is a change in default behavior.

Before we merge this, i want to make sure we're all align on the deployment for apache/iceberg-rest-fixture.

Once we merge this PR, it'll be build as the latest tag and also be picked up as the 1.10 tag when we release 1.10. I think we should call out this behavior change in the 1.10 release notes. And perhaps also send out a devlist note to callout this change, it'll be searchable for future reference :)

@kevinjqliu kevinjqliu added this to the Iceberg 1.10.0 milestone Jul 24, 2025
@kevinjqliu kevinjqliu added the release notes PR should be included in the release notes label Jul 24, 2025
@stevenzwu
Copy link
Contributor

It makes sense to align the text fixture to the REST spec where NoSuchNamespaceError is expected in this case.

@stevenzwu stevenzwu merged commit 43bd9c3 into apache:main Jul 25, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes PR should be included in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants