You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/08/31 03:32:28 UTC
[GitHub] [pulsar] congbobo184 opened a new pull request #11856: [Schema] Schema compatibility strategy in broker level.
congbobo184 opened a new pull request #11856:
URL: https://github.com/apache/pulsar/pull/11856
## Motivation
link #11849
Schema compatibility strategy config in broker level.
## implement
If namespace schema compatibility strategy is `UNDEFINED`, use broker schema compatibility strategy
### Verifying this change
Add the tests for it
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)
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] congbobo184 commented on a change in pull request #11856: [Schema] Schema compatibility strategy in broker level.
Posted by GitBox <gi...@apache.org>.
congbobo184 commented on a change in pull request #11856:
URL: https://github.com/apache/pulsar/pull/11856#discussion_r698966658
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -1955,6 +1957,13 @@
"org.apache.pulsar.broker.service.schema.ProtobufNativeSchemaCompatibilityCheck"
);
+ @FieldContext(
+ category = CATEGORY_SCHEMA,
+ doc = "The schema compatibility strategy in broker level. If this config in namespace policy is `UNDEFINED`"
+ + ", schema compatibility strategy check will use it in broker level."
+ )
+ private String schemaCompatibilityStrategy = "UNDEFINED";
Review comment:
good idea
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #11856: [Schema] Schema compatibility strategy in broker level.
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #11856:
URL: https://github.com/apache/pulsar/pull/11856#issuecomment-997825050
@eolivelli This PR is already in branch-2.9 and will be released in 2.9.1. So I remove the label `release/2.9.2`
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat commented on a change in pull request #11856: [Schema] Schema compatibility strategy in broker level.
Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #11856:
URL: https://github.com/apache/pulsar/pull/11856#discussion_r698987994
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -1955,6 +1957,13 @@
"org.apache.pulsar.broker.service.schema.ProtobufNativeSchemaCompatibilityCheck"
);
+ @FieldContext(
+ category = CATEGORY_SCHEMA,
+ doc = "The schema compatibility strategy in broker level. If this config in namespace policy is `UNDEFINED`"
+ + ", schema compatibility strategy check will use it in broker level."
+ )
+ private String schemaCompatibilityStrategy = "UNDEFINED";
Review comment:
We should be able to directly use the enum type here
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on a change in pull request #11856: [Schema] Schema compatibility strategy in broker level.
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on a change in pull request #11856:
URL: https://github.com/apache/pulsar/pull/11856#discussion_r698959315
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -1955,6 +1957,13 @@
"org.apache.pulsar.broker.service.schema.ProtobufNativeSchemaCompatibilityCheck"
);
+ @FieldContext(
+ category = CATEGORY_SCHEMA,
+ doc = "The schema compatibility strategy in broker level. If this config in namespace policy is `UNDEFINED`"
+ + ", schema compatibility strategy check will use it in broker level."
+ )
+ private String schemaCompatibilityStrategy = "UNDEFINED";
Review comment:
Could we use the name of the enum `SchemaCompatibilityStrategy.UNDEFINED`.
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
##########
@@ -545,8 +545,11 @@ public void recordAddLatency(long latency, TimeUnit unit) {
protected void setSchemaCompatibilityStrategy (Policies policies) {
if (policies.schema_compatibility_strategy == SchemaCompatibilityStrategy.UNDEFINED) {
- schemaCompatibilityStrategy = SchemaCompatibilityStrategy.fromAutoUpdatePolicy(
- policies.schema_auto_update_compatibility_strategy);
+ schemaCompatibilityStrategy = SchemaCompatibilityStrategy.valueOf(brokerService.pulsar()
+ .getConfig().getSchemaCompatibilityStrategy());
+ if (schemaCompatibilityStrategy == SchemaCompatibilityStrategy.UNDEFINED) {
Review comment:
Could we use the equals method of the enum `SchemaCompatibilityStrategy.UNDEFINED`.
such as
```
SchemaCompatibilityStrategy.UNDEFINED.equals(schemaCompatibilityStrategy)
```
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
##########
@@ -545,8 +545,11 @@ public void recordAddLatency(long latency, TimeUnit unit) {
protected void setSchemaCompatibilityStrategy (Policies policies) {
if (policies.schema_compatibility_strategy == SchemaCompatibilityStrategy.UNDEFINED) {
- schemaCompatibilityStrategy = SchemaCompatibilityStrategy.fromAutoUpdatePolicy(
- policies.schema_auto_update_compatibility_strategy);
+ schemaCompatibilityStrategy = SchemaCompatibilityStrategy.valueOf(brokerService.pulsar()
+ .getConfig().getSchemaCompatibilityStrategy());
+ if (schemaCompatibilityStrategy == SchemaCompatibilityStrategy.UNDEFINED) {
Review comment:
Ok
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] congbobo184 merged pull request #11856: [Schema] Schema compatibility strategy in broker level.
Posted by GitBox <gi...@apache.org>.
congbobo184 merged pull request #11856:
URL: https://github.com/apache/pulsar/pull/11856
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] congbobo184 commented on a change in pull request #11856: [Schema] Schema compatibility strategy in broker level.
Posted by GitBox <gi...@apache.org>.
congbobo184 commented on a change in pull request #11856:
URL: https://github.com/apache/pulsar/pull/11856#discussion_r698966658
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -1955,6 +1957,13 @@
"org.apache.pulsar.broker.service.schema.ProtobufNativeSchemaCompatibilityCheck"
);
+ @FieldContext(
+ category = CATEGORY_SCHEMA,
+ doc = "The schema compatibility strategy in broker level. If this config in namespace policy is `UNDEFINED`"
+ + ", schema compatibility strategy check will use it in broker level."
+ )
+ private String schemaCompatibilityStrategy = "UNDEFINED";
Review comment:
good idea
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
##########
@@ -545,8 +545,11 @@ public void recordAddLatency(long latency, TimeUnit unit) {
protected void setSchemaCompatibilityStrategy (Policies policies) {
if (policies.schema_compatibility_strategy == SchemaCompatibilityStrategy.UNDEFINED) {
- schemaCompatibilityStrategy = SchemaCompatibilityStrategy.fromAutoUpdatePolicy(
- policies.schema_auto_update_compatibility_strategy);
+ schemaCompatibilityStrategy = SchemaCompatibilityStrategy.valueOf(brokerService.pulsar()
+ .getConfig().getSchemaCompatibilityStrategy());
+ if (schemaCompatibilityStrategy == SchemaCompatibilityStrategy.UNDEFINED) {
Review comment:
Because it compares enums, we don't need to use equals
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #11856: [Schema] Schema compatibility strategy in broker level.
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #11856:
URL: https://github.com/apache/pulsar/pull/11856#issuecomment-909771837
Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #11856: [Schema] Schema compatibility strategy in broker level.
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #11856:
URL: https://github.com/apache/pulsar/pull/11856#issuecomment-909771837
Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on a change in pull request #11856: [Schema] Schema compatibility strategy in broker level.
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on a change in pull request #11856:
URL: https://github.com/apache/pulsar/pull/11856#discussion_r698959315
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -1955,6 +1957,13 @@
"org.apache.pulsar.broker.service.schema.ProtobufNativeSchemaCompatibilityCheck"
);
+ @FieldContext(
+ category = CATEGORY_SCHEMA,
+ doc = "The schema compatibility strategy in broker level. If this config in namespace policy is `UNDEFINED`"
+ + ", schema compatibility strategy check will use it in broker level."
+ )
+ private String schemaCompatibilityStrategy = "UNDEFINED";
Review comment:
Could we use the name of the enum `SchemaCompatibilityStrategy.UNDEFINED`.
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
##########
@@ -545,8 +545,11 @@ public void recordAddLatency(long latency, TimeUnit unit) {
protected void setSchemaCompatibilityStrategy (Policies policies) {
if (policies.schema_compatibility_strategy == SchemaCompatibilityStrategy.UNDEFINED) {
- schemaCompatibilityStrategy = SchemaCompatibilityStrategy.fromAutoUpdatePolicy(
- policies.schema_auto_update_compatibility_strategy);
+ schemaCompatibilityStrategy = SchemaCompatibilityStrategy.valueOf(brokerService.pulsar()
+ .getConfig().getSchemaCompatibilityStrategy());
+ if (schemaCompatibilityStrategy == SchemaCompatibilityStrategy.UNDEFINED) {
Review comment:
Could we use the equals method of the enum `SchemaCompatibilityStrategy.UNDEFINED`.
such as
```
SchemaCompatibilityStrategy.UNDEFINED.equals(schemaCompatibilityStrategy)
```
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] congbobo184 merged pull request #11856: [Schema] Schema compatibility strategy in broker level.
Posted by GitBox <gi...@apache.org>.
congbobo184 merged pull request #11856:
URL: https://github.com/apache/pulsar/pull/11856
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat commented on a change in pull request #11856: [Schema] Schema compatibility strategy in broker level.
Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #11856:
URL: https://github.com/apache/pulsar/pull/11856#discussion_r698987994
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -1955,6 +1957,13 @@
"org.apache.pulsar.broker.service.schema.ProtobufNativeSchemaCompatibilityCheck"
);
+ @FieldContext(
+ category = CATEGORY_SCHEMA,
+ doc = "The schema compatibility strategy in broker level. If this config in namespace policy is `UNDEFINED`"
+ + ", schema compatibility strategy check will use it in broker level."
+ )
+ private String schemaCompatibilityStrategy = "UNDEFINED";
Review comment:
We should be able to directly use the enum type here
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] congbobo184 commented on a change in pull request #11856: [Schema] Schema compatibility strategy in broker level.
Posted by GitBox <gi...@apache.org>.
congbobo184 commented on a change in pull request #11856:
URL: https://github.com/apache/pulsar/pull/11856#discussion_r698967046
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
##########
@@ -545,8 +545,11 @@ public void recordAddLatency(long latency, TimeUnit unit) {
protected void setSchemaCompatibilityStrategy (Policies policies) {
if (policies.schema_compatibility_strategy == SchemaCompatibilityStrategy.UNDEFINED) {
- schemaCompatibilityStrategy = SchemaCompatibilityStrategy.fromAutoUpdatePolicy(
- policies.schema_auto_update_compatibility_strategy);
+ schemaCompatibilityStrategy = SchemaCompatibilityStrategy.valueOf(brokerService.pulsar()
+ .getConfig().getSchemaCompatibilityStrategy());
+ if (schemaCompatibilityStrategy == SchemaCompatibilityStrategy.UNDEFINED) {
Review comment:
Because it compares enums, we don't need to use equals
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on a change in pull request #11856: [Schema] Schema compatibility strategy in broker level.
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on a change in pull request #11856:
URL: https://github.com/apache/pulsar/pull/11856#discussion_r698986293
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
##########
@@ -545,8 +545,11 @@ public void recordAddLatency(long latency, TimeUnit unit) {
protected void setSchemaCompatibilityStrategy (Policies policies) {
if (policies.schema_compatibility_strategy == SchemaCompatibilityStrategy.UNDEFINED) {
- schemaCompatibilityStrategy = SchemaCompatibilityStrategy.fromAutoUpdatePolicy(
- policies.schema_auto_update_compatibility_strategy);
+ schemaCompatibilityStrategy = SchemaCompatibilityStrategy.valueOf(brokerService.pulsar()
+ .getConfig().getSchemaCompatibilityStrategy());
+ if (schemaCompatibilityStrategy == SchemaCompatibilityStrategy.UNDEFINED) {
Review comment:
Ok
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org