yuqi1129 commented on PR #13382: URL: https://github.com/apache/gravitino/pull/13382#issuecomment-5770707335
Thanks for the thorough doc update. I checked the REST paths, request bodies, Java signatures, privilege names, lookup semantics (disabled filtering, `inherited`, dedupe) and every intra-doc anchor against the current `main`; they all match. A few things to fix before merging: **1. `supportedObjectTypes` immutability is only enforced for built-in policies** `docs/policies.md#L49` and `docs/manage-policies-in-gravitino.md#L25` say `supportedObjectTypes` cannot be changed after creation. In `PolicyManager.updatePolicyEntity` the guard is `if (policyType != Policy.BuiltInType.CUSTOM)`, so a custom policy's `supportedObjectTypes` *can* be replaced via `UpdateContent`. Suggest scoping the statement to built-in types, or noting that a custom content update replaces the whole content including this field. **2. Tag override rule is narrower than the implementation** `docs/policies.md#L73-L74` and `docs/migration-guide.md#L60` describe only "the object's direct assignment wins". `EffectiveTagResolver` walks from the object upward with first-assignment-wins, so a nearer ancestor also overrides a farther one — e.g. `data_domain=risk` on a schema overrides `data_domain=finance` on the catalog for every table in that schema. Readers planning `TAG_VALUE` selectors should know this; suggest "a direct or nearer assignment overrides a farther one". **3. Orphaned asset** Removing the "Policy Metadata Objects" section leaves `docs/assets/webui-v2/policy-metadata-objects.png` unreferenced. Please delete it in this PR. **4. (nit) UI state described inconsistently** `docs/migration-guide.md#L77` says the web UI still shows direct policy controls that hit the removed write endpoint (true — `web-v2/web/src/lib/api/policies/index.js` still has the `ASSOCIATE` route and the `metadataObjectsForPolicy` page). `docs/webui-v2.md#L130` reads as if those controls are already gone. Suggest a short note in `webui-v2.md` that the control is currently non-functional pending UI cleanup, so the two pages agree; the UI cleanup itself can be a follow-up issue. Otherwise LGTM. -- 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]
