Catalog tests run on Polaris on CI#3106
Conversation
rambleraptor
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks @rambleraptor, this is a cool idea, took me a bit to think this one through. Polaris moves fast, and having it in CI means we automatically validate against a robust REST catalog implementation not waiting on rck changes. I'm looking forward to this being the foundation for testing scan planning and vended credentials once that path is finalized.
Right now this adds the full e2e workflow to run the basic catalog tests against Polaris, which is a good start that we can stand up Polaris and run against it even outside of testing. I also wanted to call out that there is a lot of value in Polaris on other fronts like credential vending, access, auth flows. Is there a plan to expand the test surface toward that?
Also, left a few comments on a first pass, and will leave open for some other comments!
| PYICEBERG_CATALOG__POLARIS__S3__SECRET_ACCESS_KEY="password" \ | ||
| PYICEBERG_CATALOG__POLARIS__S3__REGION="us-east-1" \ | ||
| $(TEST_RUNNER) pytest tests/integration/test_catalog.py -k "rest_test_catalog and not test_update_namespace_properties" $(PYTEST_ARGS) | ||
| # Skip test_update_namespace_properties: Polaris triggers a CommitConflictException when updates and removals are in the same request. |
There was a problem hiding this comment.
Is this a known Polaris bug or intentional behavior? Is there a tracking issue against this? We should link for future context.
|
|
||
| test-polaris-exec: ## Run Polaris integration tests | ||
| @eval $$(cat dev/polaris_creds.env) && \ | ||
| PYICEBERG_TEST_CATALOG="polaris" \ |
There was a problem hiding this comment.
Not sure if I like populating the PyIceberg configs here and convoluting the MF. this makes for a cool case to have a persistent .pyiceberg.yaml in /dev and have all tests point there lol
| $(TEST_RUNNER) pytest tests/integration/test_catalog.py -k "rest_test_catalog and not test_update_namespace_properties" $(PYTEST_ARGS) | ||
| # Skip test_update_namespace_properties: Polaris triggers a CommitConflictException when updates and removals are in the same request. | ||
|
|
||
|
|
2 participants
Rationale for this change
Thanks to #2482, we can run our catalog tests against an arbitrary REST Catalog using env variables.
This PR wires up Polaris on our CI and runs all of our catalog tests against Polaris. This should help a lot with any future regressions.
One test is disabled, since Polaris doesn't seem to like having updates/removals in a single REST call. Not sure if this is intentional or not.
Are these changes tested?
Just tests
Are there any user-facing changes?
New PRs will spin up Polaris + run tests.