nevzheng commented on PR #12132: URL: https://github.com/apache/gravitino/pull/12132#issuecomment-5051304083
Thanks for the review feedback. I’ve completed the author-side revisions on the current PR: - Moved the provider SPI, exceptions, and reusable test contracts from `api` to `common`; `api` retains the public `KmsReference` value. - Replaced the closed `KmsApi` enum with canonical string identifiers so independently loaded providers can extend the SPI safely. - Clarified that `KmsKeyProperties` describes one inspected key, not client- or provider-level capabilities. - Changed key lookup to return `Optional<KmsKeyProperties>`: empty means authoritative not-found, while authentication, authorization, availability, and indeterminate failures remain exceptions. - Documented that lifecycle state is point-in-time and may apply to the selected/current key version; operation support does not guarantee authorization or future success. - Added contract coverage for canonical identifiers, found/missing/disabled keys, invalid provider results, and distinct failure types. Local API/common tests, Spotless, test-fixture packaging, and Javadocs pass. GitHub CI currently has no failures; the remaining integration jobs are still running. Current head: `cf4a8c976` @roryqi @lasdf1234 — please take another look and either approve or identify any remaining concrete concerns. -- 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]
