You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "mattfaltyn (via GitHub)" <gi...@apache.org> on 2026/08/01 19:00:10 UTC
[PR] Fix setup export policy name collisions [polaris]
mattfaltyn opened a new pull request, #5216:
URL: https://github.com/apache/polaris/pull/5216
## Summary
- export policies as a list with explicit `name` and `namespace` fields so namespace-scoped identities cannot collide
- keep `setup apply` compatible with existing policy mappings keyed by name
- cover an export/YAML/apply round trip with duplicate policy names in different namespaces
- document the exported representation and record the user-visible fix in the changelog
## Rationale
Policy names are unique only within a namespace. The previous catalog-wide mapping used policy name as its sole key, so a later namespace silently replaced an earlier same-named policy and made setup backups incomplete.
Fixes #5215
## Validation
- `client/python/.venv/bin/pytest client/python/tests/test_setup_command.py -q` (`18 passed, 3 subtests passed`)
- `.venv/bin/pytest tests/` from `client/python` (`171 passed`)
- repository client pre-commit hooks: trailing whitespace, EOF, debug statements, Ruff check/format, and mypy (all passed)
## AI disclosure
This change was developed with Codex assistance. I reviewed the implementation, tests, documentation, and generated diff and take responsibility for the contribution.
## Checklist
- [x] 🛡️ This is a public, non-security correctness fix; no security issue is disclosed
- [x] 🔗 The need is explained above and linked with `Fixes #5215`
- [x] 🧪 Added a regression test covering the failing export/apply round trip
- [x] 💡 The normalization is direct and does not require additional explanatory comments
- [x] 🧾 Updated `CHANGELOG.md`
- [x] 📚 Updated documentation in `site/content/in-dev/unreleased`
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix setup export policy name collisions [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on code in PR #5216:
URL: https://github.com/apache/polaris/pull/5216#discussion_r3741463271
##########
site/content/guides/assets/polaris/reference-setup-config.yaml:
##########
@@ -110,8 +110,9 @@ catalogs:
# --- Policies ---
# Defines policies that can be attached to entities in this catalog.
+ # Use namespace: "" for a catalog-root policy.
Review Comment:
This is not correct I think based on what @flyrain shared?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix setup export policy name collisions [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #5216:
URL: https://github.com/apache/polaris/pull/5216#issuecomment-5172915117
> * policy can be catalog level. For the export to include all, we would need to include case where namespace is `""` (we should update `command-line-interface.md` as well).
You can assign a policy to a catalog, but a policy is always hosted within a namespace, https://github.com/apache/polaris/blob/eb826c2ad5c8e4a6da14675dc1bc94b1f1878583/spec/polaris-catalog-apis/policy-apis.yaml#L23.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix setup export policy name collisions [polaris]
Posted by "mattfaltyn (via GitHub)" <gi...@apache.org>.
mattfaltyn commented on code in PR #5216:
URL: https://github.com/apache/polaris/pull/5216#discussion_r3755977418
##########
client/python/tests/test_setup_command.py:
##########
@@ -438,8 +439,12 @@ def test_setup_exported_policy_content_round_trips_through_apply(
) -> None:
policy_content = '{"version":"2025-02-03","enable":true}'
policy_api = mock_policy_api_class.return_value
- policy_api.list_policies.return_value = SimpleNamespace(
- identifiers=[SimpleNamespace(name="compaction")]
+ policy_api.list_policies.side_effect = lambda prefix, namespace: SimpleNamespace(
Review Comment:
Good call - simplified this back to return_value.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix setup export policy name collisions [polaris]
Posted by "mattfaltyn (via GitHub)" <gi...@apache.org>.
mattfaltyn commented on code in PR #5216:
URL: https://github.com/apache/polaris/pull/5216#discussion_r3741490609
##########
site/content/guides/assets/polaris/reference-setup-config.yaml:
##########
@@ -110,8 +110,9 @@ catalogs:
# --- Policies ---
# Defines policies that can be attached to entities in this catalog.
+ # Use namespace: "" for a catalog-root policy.
Review Comment:
Good catch—removed. Policies can attach to catalogs, but they are always hosted in namespaces.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix setup export policy name collisions [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode merged PR #5216:
URL: https://github.com/apache/polaris/pull/5216
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix setup export policy name collisions [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on PR #5216:
URL: https://github.com/apache/polaris/pull/5216#issuecomment-5262463636
@mattfaltyn mind fix the conflict here?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix setup export policy name collisions [polaris]
Posted by "mattfaltyn (via GitHub)" <gi...@apache.org>.
mattfaltyn commented on PR #5216:
URL: https://github.com/apache/polaris/pull/5216#issuecomment-5225500858
Final-review correction: removed catalog-root policy export/apply behavior. As @flyrain clarified, policies are hosted in namespaces; attachment to a catalog does not make the catalog root a policy host. The PR now stays focused on preserving duplicate names across namespaces, with the canonical list format, reference YAML, legacy-format limitation, and regression coverage retained.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix setup export policy name collisions [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on code in PR #5216:
URL: https://github.com/apache/polaris/pull/5216#discussion_r3755013167
##########
client/python/tests/test_setup_command.py:
##########
@@ -438,8 +439,12 @@ def test_setup_exported_policy_content_round_trips_through_apply(
) -> None:
policy_content = '{"version":"2025-02-03","enable":true}'
policy_api = mock_policy_api_class.return_value
- policy_api.list_policies.return_value = SimpleNamespace(
- identifiers=[SimpleNamespace(name="compaction")]
+ policy_api.list_policies.side_effect = lambda prefix, namespace: SimpleNamespace(
Review Comment:
This is not necessary perhaps?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix setup export policy name collisions [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #5216:
URL: https://github.com/apache/polaris/pull/5216#discussion_r3770336602
##########
CHANGELOG.md:
##########
@@ -46,6 +46,9 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
### Fixes
+- Python CLI `setup export` now preserves policies with the same name in different namespaces.
Review Comment:
This says collisions are fixed but not that the export *format* changed. `policies` used to be a name-keyed mapping and is now a list of `{name, namespace, ...}`, so an export produced by this version can't be applied by a released CLI. Can we clarify it in the changelog so operators mixing CLI versions aren't surprised?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix setup export policy name collisions [polaris]
Posted by "mattfaltyn (via GitHub)" <gi...@apache.org>.
mattfaltyn commented on PR #5216:
URL: https://github.com/apache/polaris/pull/5216#issuecomment-5225220095
Addressed all review feedback in the rebased update: catalog-root policies now round-trip with an explicit empty namespace, the reference setup YAML uses the canonical policy list with explicit names, and the retained legacy mapping format is documented as unable to represent duplicate policy names.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix setup export policy name collisions [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on PR #5216:
URL: https://github.com/apache/polaris/pull/5216#issuecomment-5244729626
Sorry for the delay, I will take a look later tonight.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix setup export policy name collisions [polaris]
Posted by "mattfaltyn (via GitHub)" <gi...@apache.org>.
mattfaltyn commented on code in PR #5216:
URL: https://github.com/apache/polaris/pull/5216#discussion_r3741490609
##########
site/content/guides/assets/polaris/reference-setup-config.yaml:
##########
@@ -110,8 +110,9 @@ catalogs:
# --- Policies ---
# Defines policies that can be attached to entities in this catalog.
+ # Use namespace: "" for a catalog-root policy.
Review Comment:
Good catch - I removed it. Policies can attach to catalogs, but they are always hosted in namespaces.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix setup export policy name collisions [polaris]
Posted by "mattfaltyn (via GitHub)" <gi...@apache.org>.
mattfaltyn commented on code in PR #5216:
URL: https://github.com/apache/polaris/pull/5216#discussion_r3741490609
##########
site/content/guides/assets/polaris/reference-setup-config.yaml:
##########
@@ -110,8 +110,9 @@ catalogs:
# --- Policies ---
# Defines policies that can be attached to entities in this catalog.
+ # Use namespace: "" for a catalog-root policy.
Review Comment:
Good catch - I removed it.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix setup export policy name collisions [polaris]
Posted by "mattfaltyn (via GitHub)" <gi...@apache.org>.
mattfaltyn commented on code in PR #5216:
URL: https://github.com/apache/polaris/pull/5216#discussion_r3773185118
##########
CHANGELOG.md:
##########
@@ -46,6 +46,9 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
### Fixes
+- Python CLI `setup export` now preserves policies with the same name in different namespaces.
Review Comment:
@flyrain Good catch - updated the changelog to document the new list format and older CLI incompatibility.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org