roryqi commented on PR #12855: URL: https://github.com/apache/gravitino/pull/12855#issuecomment-5539706231
@yuqi1129 Thanks for the detailed review. I addressed the blocking issues in `ffabdc7d0`. - Pre-events now contain mutation intent only and are emitted before any business access. - The non-atomic association snapshot and O(N) relation scan have been removed, along with `PolicyTagAssociationInfo`. - Success events no longer claim previous, resulting, or removed state. They describe the successfully completed request. Since add is create-only and remove is idempotent with a void contract, the earlier concurrency and point-selection concerns no longer apply to the event payload. If actual mutation snapshots are needed later, I agree they should come from a transaction-level mutation result rather than a separate read. - The public contract is now consistently create-only: `POST` returns `201`, and duplicate associations conflict regardless of selector. Tests cover duplicates with both the same and a different selector. - Pre-event ordering, listener veto, delegate invocation, and failure propagation are covered by tests. - The v2 audit payload now contains `policyName` and the requested selector for add, with actual formatter-output coverage. The v1 limitation is documented and tested. I also replied to each inline thread with the corresponding change. Could you please take another look, especially at the reduced intent-only event contract? -- 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]
