Skip to content
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

[Issue 12757][broker] add broker config isAllowAutoUpdateSchema #12786

Merged

Conversation

Jason918
Copy link
Contributor

@Jason918 Jason918 commented Nov 13, 2021

Fixes #12757

Motivation

See #12757

Modifications

Add a broker config : isAllowAutoUpdateSchema

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • org.apache.pulsar.schema.compatibility.SchemaCompatibilityCheckTest#testBrokerAllowAutoUpdateSchemaDisabled

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: (yes)
  • 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

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

  • doc

Added doc in the annotation of new config.

@github-actions github-actions bot added the doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. label Nov 13, 2021
@Jason918
Copy link
Contributor Author

@codelipenghui PTAL

@Jason918
Copy link
Contributor Author

/pulsarbot run-failure-checks

Copy link
Member

@shoothzj shoothzj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think allowAutoUpdateSchema is better. the boolean type of field have implied the meaning of is

@Jason918
Copy link
Contributor Author

I think allowAutoUpdateSchema is better. the boolean type of field have implied the meaning of is

Nice catch.
And it's consistent with previous 'allowXXX' fields.
@shoothzj Updated, please help review again.

Copy link
Contributor

@yuruguo yuruguo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also add this config to conf/broker.conf, conf/standalone.conf and releated document?

@codelipenghui codelipenghui added this to the 2.10.0 milestone Nov 14, 2021
@Jason918
Copy link
Contributor Author

Should we also add this config to conf/broker.conf, conf/standalone.conf and releated document?

Thanks for the reminding, added the config to these files. Please take a look.

About the user doc, the config doc should be auto-generated by the annotation, right ?

@Jason918
Copy link
Contributor Author

/pulsarbot run-failure-checks

Copy link
Member

@RobertIndie RobertIndie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

conf/broker.conf Outdated Show resolved Hide resolved
@Jason918
Copy link
Contributor Author

/pulsarbot run-failure-checks

@Jason918 Jason918 force-pushed the support_broker_isSchemaAutoUploadEnabled branch from c2c6c5e to 3eddf8a Compare November 18, 2021 08:59
conf/broker.conf Outdated Show resolved Hide resolved
@Jason918
Copy link
Contributor Author

/pulsarbot run-failure-checks

@codelipenghui codelipenghui merged commit fa7be23 into apache:master Nov 18, 2021
codelipenghui pushed a commit that referenced this pull request Nov 18, 2021
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Nov 18, 2021
zeo1995 pushed a commit to zeo1995/pulsar that referenced this pull request Nov 18, 2021
* up/master:
  [Issue 12757][broker] add broker config isAllowAutoUpdateSchema (apache#12786)
zeo1995 pushed a commit to zeo1995/pulsar that referenced this pull request Nov 18, 2021
* up/master:
  [Issue 12757][broker] add broker config isAllowAutoUpdateSchema (apache#12786)
  Update deploy-bare-metal.md (apache#12432)
  [Broker] Fix producer getting incorrectly removed from topic's producers map (apache#12846)
  Add error log when new jetty client (apache#12840)
  JavaInstanceTest should be AssertEquals (apache#12836)
  [Transaction] Fix transaction flaky test testMaxReadPositionForNormalPublish (apache#12681)
  The problem of two exception handling (apache#12744)
  Fix TopicPoliciesCacheNotInitException issue. (apache#12773)
  Added local filesystem backend for package manager (apache#12708)
eolivelli pushed a commit to eolivelli/pulsar that referenced this pull request Nov 29, 2021
fxbing pushed a commit to fxbing/pulsar that referenced this pull request Dec 19, 2021
codelipenghui pushed a commit that referenced this pull request Dec 21, 2021
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Dec 21, 2021
momo-jun added a commit to momo-jun/pulsar that referenced this pull request Aug 4, 2022
momo-jun added a commit to momo-jun/pulsar that referenced this pull request Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. release/2.8.3 release/2.9.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support enable or disable schema upload at the broker level
10 participants