richiebachala commented on PR #2190: URL: https://github.com/apache/iceberg-go/pull/2190#issuecomment-6295804072
Polaris sends a /v1/config prefix & a namespace-separator override, requires OAuth (it rejects the default catalog scope), and rejects self-rename with a 409 where the fixture returns success. Those client paths are already covered by unit tests with fake servers, and the suite only uses single-level namespaces, so the separator isn't exercised here at all. What Polaris adds is a real server exercising prefix and OAuth end to end, which the unauthenticated fixture never does. Useful, but not a gap the fixture leaves in code coverage. I agree it shouldn't be a Polaris-specific test. 2 options: 1. Generalize it: an opt-in TestExternalRestCatalogConformance pointed at any REST catalog via REST_CATALOG_URI/_CREDENTIAL/_WAREHOUSE, with Polaris as a documented example, so contributors can check Lakekeeper, Nessie, and others without per-catalog tests. 2. Close this PR and keep the suite on the fixture only. I'm happy with either; I lean toward (1) since it's small and catalog-neutral. @laskoviymishka, does that work for your CI concern? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
