-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Add schema compatibility strategy on topic level #13297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add schema compatibility strategy on topic level #13297
Conversation
|
@nodece:Thanks for your contribution. For this PR, do we need to update docs? |
|
Hi @nodece please do not delete backticks (for example, |
76233bc to
087502c
Compare
|
@Jason918 Could you please help review this PR? Please make sure the new change follows for enhancement way. |
c6c25b8 to
d8a84e4
Compare
|
/pulsarbot run-failure-checks |
Jason918
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please refer to org.apache.pulsar.broker.service.AbstractTopic#topicPolicies, and try to make it consistent with other policy handling.
pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Topics.java
Outdated
Show resolved
Hide resolved
pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/TopicPolicies.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
Outdated
Show resolved
Hide resolved
pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/TopicPolicies.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
Outdated
Show resolved
Hide resolved
...r/src/test/java/org/apache/pulsar/schema/compatibility/SchemaTypeCompatibilityCheckTest.java
Outdated
Show resolved
Hide resolved
...r/src/test/java/org/apache/pulsar/schema/compatibility/SchemaTypeCompatibilityCheckTest.java
Outdated
Show resolved
Hide resolved
|
/pulsarbot run-failure-checks |
2 similar comments
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/SchemasResourceBase.java
Outdated
Show resolved
Hide resolved
...ava/org/apache/pulsar/schema/compatibility/SchemaTypeCompatibilityCheckOnTopicLevelTest.java
Outdated
Show resolved
Hide resolved
|
/pulsarbot run-failure-checks |
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java
Outdated
Show resolved
Hide resolved
...r/src/test/java/org/apache/pulsar/schema/compatibility/SchemaTypeCompatibilityCheckTest.java
Show resolved
Hide resolved
8c524d5 to
1e26beb
Compare
24dd4f1 to
1a1d0a5
Compare
|
/pulsarbot run-failure-checks |
|
@codelipenghui @congbobo184 @Jason918 @315157973 could you take a look this PR? |
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
Outdated
Show resolved
Hide resolved
42cfa6e to
498f9de
Compare
f40705f to
332ad9b
Compare
|
/pulsarbot run-failure-checks |
a19abda to
8268c38
Compare
### Motivation Currently, the compatibility strategy for a schema is set at namespace level, which means that all the topics of a namespace will need to adhere to the same compatibility check, this level particle size is relatively large, so add schema compatibility strategy on topic level. ### Modifications - Add set, get and remove schema compatibility strategy API to admin API and client tool - Add schema compatibility strategy action to TopicOperation ### Verifying this change - [x] Make sure that the change passes the CI checks. ### Does this pull request potentially affect one of the following parts: *If `yes` was chosen, please highlight the changes* - Dependencies (does it add or upgrade a dependency): no - The public API: no - The schema: no - The default values of configurations: no - The wire protocol: no - The rest endpoints: no - The admin cli options: no - Anything that affects deployment: no ### Documentation - [x] `doc-required` Signed-off-by: Zixuan Liu <nodeces@gmail.com>
8268c38 to
4053e29
Compare
congbobo184
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! nice work
|
@Jason918 please review again, thanks. |
Jason918
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a small comment, LGTM overall.
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
### Motivation Currently, the compatibility strategy for a schema is set at namespace level, which means that all the topics of a namespace will need to adhere to the same compatibility check, this level particle size is relatively large, so add schema compatibility strategy on topic level. ### Modifications - Add set, get and remove schema compatibility strategy API to admin API and client tool - Add schema compatibility strategy action to TopicOperation
Motivation
Currently, the compatibility strategy for a schema is set at namespace level, which means that all the topics of a namespace will need to adhere to the same compatibility check, this level particle size is relatively large, so add schema compatibility strategy on topic level.
Modifications
Verifying this change
Does this pull request potentially affect one of the following parts:
If
yeswas chosen, please highlight the changesDocumentation
Check the box below and label this PR (if you have committer privilege).
Need to update docs?
doc-requiredno-need-docdoc