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

[Python Schema] Fix python schema array map with record #11530

Merged

Conversation

gaoran10
Copy link
Contributor

@gaoran10 gaoran10 commented Aug 2, 2021

Motivation

Currently, the schema type Array and Map couldn't work well with Record.

For example.

class NestedObj(Record):
    na2 = Integer()
    nb2 = Boolean()

class ComplexRecord(Record):
    a = Integer()
    b = Integer()
    arrayNested = Array(NestedObj())

nested_obj = NestedObj(na2=22, nb2=True)
r = ComplexRecord(a=1, b=2, arrayNested=[
    nested_obj, nested_obj, nested_obj
])

error log

  File "/Volumes/shit/Workspaces/GitHubFork/pulsar/pulsar-client-cpp/python/pulsar/schema/definition.py", line 74, in __init__
    self.__setattr__(k, kwargs[k])
  File "/Volumes/shit/Workspaces/GitHubFork/pulsar/pulsar-client-cpp/python/pulsar/schema/definition.py", line 117, in __setattr__
    value = field.validate_type(key, value)
  File "/Volumes/shit/Workspaces/GitHubFork/pulsar/pulsar-client-cpp/python/pulsar/schema/definition.py", line 369, in validate_type
    if type(x) != self.array_type.python_type():
AttributeError: 'NestedObj2' object has no attribute 'python_type'

Modifications

Change the method __init__() of the Record, because decoding data will depend on this method, so we need to use dict data to initialize the Record object.

Add a new method python_type() for Record, the schema type Array and Map need to get python_type of the Record.

Verifying this change

Add test to verify schema type Array and Map work with Record.

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

For committer

For this PR, do we need to update docs?

  • If yes,

    • if you update docs in this PR, label this PR with the doc label.

    • if you plan to update docs later, label this PR with the doc-required label.

    • if you need help on updating docs, create a follow-up issue with the doc-required label.

  • If no, label this PR with the no-need-doc label and explain why.

@merlimat merlimat merged commit 03aedc7 into apache:master Aug 2, 2021
@gaoran10 gaoran10 deleted the fix-python-schema-array-map-with-record branch August 3, 2021 15:57
codelipenghui pushed a commit that referenced this pull request Aug 4, 2021
* fix Python schema type Array and Map work with Record

* fix test

(cherry picked from commit 03aedc7)
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Aug 4, 2021
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
* fix Python schema type Array and Map work with Record

* fix test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-2.8 Archived: 2.8 is end of life release/2.8.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants