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

[fix][python client] Fixed reserved keys is not removed when JsonSchema being encoded #15947

Merged
merged 1 commit into from
Jun 7, 2022

Conversation

boatrainlsz
Copy link
Contributor

@boatrainlsz boatrainlsz commented Jun 6, 2022

Fixes #15844

Motivation

JsonSchema doesn't remove reserved keys(_default,_required,_required_default) when being encoded. Resulting in confusing encoding output.

Modifications

Extract a common method to remove reserved keys when JsonSchema is being encoded, and this common method will be called when JsonSchema::encode method is called.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • Added a unit test method named test_json_schema_encode_remove_reserved_key, see pulsar-client-cpp/python/schema_test.py

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): (yes / no)
  • The public API: (yes / no)
  • The schema: (yes / no / don't know)
  • The default values of configurations: (yes / no)
  • The wire protocol: (yes / no)
  • The rest endpoints: (yes / no)
  • The admin cli options: (yes / no)
  • Anything that affects deployment: (yes / no / don't know)

Documentation

Check the box below or label this PR directly.

Need to update docs?

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

@github-actions
Copy link

github-actions bot commented Jun 6, 2022

@boatrainlsz:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@github-actions github-actions bot added doc-label-missing doc-complete Your PR changes impact docs and the related docs have been already added. labels Jun 6, 2022
Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

The change looks good. Please remove the unintended changes to the config files.

@boatrainlsz
Copy link
Contributor Author

The change looks good. Please remove the unintended changes to the config files.

Sorry for that, I will remove those unintended changes later

@boatrainlsz
Copy link
Contributor Author

The change looks good. Please remove the unintended changes to the config files.

Sorry for that, I will remove those unintended changes later

@merlimat I've removed those changes, please kindly review it.

@Anonymitaet Anonymitaet added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing doc-complete Your PR changes impact docs and the related docs have been already added. labels Jun 7, 2022
@merlimat merlimat added this to the 2.11.0 milestone Jun 7, 2022
@merlimat merlimat added release/2.10.1 type/bug The PR fixed a bug or issue reported a bug component/client-python labels Jun 7, 2022
@boatrainlsz
Copy link
Contributor Author

/pulsarbot run-failure-checks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-2.10 doc-not-needed Your PR changes do not impact docs release/2.10.1 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python Client doesn't clean up reserved field when publishing message with Schema
5 participants