[#7552] feat(policy): Add policy entity and POConverters (part-2)#7361
[#7552] feat(policy): Add policy entity and POConverters (part-2)#7361jerryshao merged 7 commits intoapache:mainfrom
Conversation
| .withName(policyPO.getPolicyName()) | ||
| .withNamespace(namespace) | ||
| .withType(policyPO.getPolicyType()) | ||
| .withComment(policyPO.getPolicyVersionPO().getPolicyComment()) |
There was a problem hiding this comment.
Do you think it should check if the policyVersion is null here?
There was a problem hiding this comment.
unnecessary, because PolicyPO.Builder already did the validation
|
This PR is too big, can you please split into several small PRs for better review? |
Split done in this PR, plz help to review when you have time, thx! @jerryshao |
core/src/main/java/org/apache/gravitino/policy/CustomPolicy.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/gravitino/storage/relational/po/PolicyPO.java
Outdated
Show resolved
Hide resolved
| * @throws IllegalPolicyException if the policy is not valid. | ||
| */ | ||
| void validate() throws IllegalPolicyException; | ||
| default void validate() throws IllegalPolicyException { |
There was a problem hiding this comment.
Who will call this validate() method?
There was a problem hiding this comment.
Currently, it's called by PolicyEntity.valiate()
There was a problem hiding this comment.
I would suggest to move this method to PolicyEntity, this seems is not a user-facing interface, or should user be aware of.
api/src/main/java/org/apache/gravitino/policy/PolicyContent.java
Outdated
Show resolved
Hide resolved
api/src/main/java/org/apache/gravitino/policy/PolicyContent.java
Outdated
Show resolved
Hide resolved
…2) (apache#7361) ### What changes were proposed in this pull request? Add policy entity and POConverters ### Why are the changes needed? Fix: apache#7552 ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? tests added
…2) (apache#7361) ### What changes were proposed in this pull request? Add policy entity and POConverters ### Why are the changes needed? Fix: apache#7552 ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? tests added
…2) (apache#7361) ### What changes were proposed in this pull request? Add policy entity and POConverters ### Why are the changes needed? Fix: apache#7552 ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? tests added
What changes were proposed in this pull request?
Add policy entity and POConverters
Why are the changes needed?
Fix: #7552
Does this PR introduce any user-facing change?
no
How was this patch tested?
tests added