-
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
[Python Schema] Support setting namespace for python schema #12175
[Python Schema] Support setting namespace for python schema #12175
Conversation
'type': 'record', | ||
'fields': [] | ||
} | ||
if schema['namespace'] is None: | ||
del schema['namespace'] |
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.
Instead of adding and then deleting, we could just add it only if it's not None
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.
Ok
@@ -57,6 +57,8 @@ def _get_fields(cls, dct): | |||
|
|||
class Record(with_metaclass(RecordMeta, object)): | |||
|
|||
_namespace = None |
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.
I was wondering given that this is very specific to Avro, if we should name it as _avro_namespace
to make it clear.
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.
One other thing is that we should add this to the Python schema docs
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.
Good idea, I'll the name _avro_namespace
.
* support set namespace for python schema * fix * fix * fix comment (cherry picked from commit f0d8fb0)
…2175) * support set namespace for python schema * fix * fix * fix comment
Motivation
Currently, the python schema didn't support setting namespace in the schema definition.
This PR is used to support setting namespace in the python schema definition.
I have two ways to set the namespace for Record, one is to add a new class method for Record to set the namespace, one is to add a new field
_avro_namespace
for class Record. I select the second way.for example, add a special field
_avro_namespace
for the custom Record to add the namespace.The schema definition is like this.
Modifications
Modify the method
schema_info
of the classRecord
to add the namespace field when generating the schema definition.Verifying this change
Add test to verify the schema definition.
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
(If you need help on updating docs, create a doc issue)
no-need-doc
(Please explain why)
doc
(If this PR contains doc changes)