-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
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 |
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
Outdated
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 <[email protected]>
8268c38
to
4053e29
Compare
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. |
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 <[email protected]>
### 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
yes
was chosen, please highlight the changesDocumentation
Check the box below and label this PR (if you have committer privilege).
Need to update docs?
doc-required
no-need-doc
doc