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/09/24 15:40:28 UTC

[GitHub] [pulsar] gaoran10 opened a new pull request #12175: [Python Schema] Support setting namespace for python schema

gaoran10 opened a new pull request #12175:
URL: https://github.com/apache/pulsar/pull/12175


   ### Motivation
   
   Currently, the python schema didn't support setting namespace in the schema definition.
   
   ### Modifications
   
   Support setting namespace in the python schema definition.
   
   for example, add a special field `_namespace` for the custom Record to set the namespace.
   ```
   class User(Record):
       _namespace = 'xxx.xxx.xxx'
       name = String()
       age = Integer()
   ```
   
   The schema definition is like this.
   ```
   {'name': 'User', 'namespace': 'xxx.xxx.xxx', 'type': 'record', 'fields': [{'name': 'age', 'type': ['null', 'int']}, {'name': 'name', 'type': ['null', 'string']}]}
   ```
   
   ### 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 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
   
   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)
   
   
   


-- 
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 #12175: [Python Schema] Support setting namespace for python schema

Posted by GitBox <gi...@apache.org>.
gaoran10 commented on a change in pull request #12175:
URL: https://github.com/apache/pulsar/pull/12175#discussion_r715968205



##########
File path: pulsar-client-cpp/python/pulsar/schema/definition.py
##########
@@ -57,6 +57,8 @@ def _get_fields(cls, dct):
 
 class Record(with_metaclass(RecordMeta, object)):
 
+    _namespace = None

Review comment:
       Good idea, I'll the name `_avro_namespace`.

##########
File path: pulsar-client-cpp/python/pulsar/schema/definition.py
##########
@@ -101,15 +103,23 @@ def schema(cls):
 
     @classmethod
     def schema_info(cls, defined_names):
-        if cls.__name__ in defined_names:
-            return cls.__name__
+        namespace_prefix = ''
+        if cls._namespace is not None:
+            namespace_prefix = cls._namespace + '.'
+        namespace_name = namespace_prefix + cls.__name__
+
+        if namespace_name in defined_names:
+            return namespace_name
 
-        defined_names.add(cls.__name__)
+        defined_names.add(namespace_name)
         schema = {
             'name': str(cls.__name__),
+            'namespace': cls._namespace,
             'type': 'record',
             'fields': []
         }
+        if schema['namespace'] is None:
+            del schema['namespace']

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] merlimat commented on a change in pull request #12175: [Python Schema] Support setting namespace for python schema

Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #12175:
URL: https://github.com/apache/pulsar/pull/12175#discussion_r715797668



##########
File path: pulsar-client-cpp/python/pulsar/schema/definition.py
##########
@@ -101,15 +103,23 @@ def schema(cls):
 
     @classmethod
     def schema_info(cls, defined_names):
-        if cls.__name__ in defined_names:
-            return cls.__name__
+        namespace_prefix = ''
+        if cls._namespace is not None:
+            namespace_prefix = cls._namespace + '.'
+        namespace_name = namespace_prefix + cls.__name__
+
+        if namespace_name in defined_names:
+            return namespace_name
 
-        defined_names.add(cls.__name__)
+        defined_names.add(namespace_name)
         schema = {
             'name': str(cls.__name__),
+            'namespace': cls._namespace,
             'type': 'record',
             'fields': []
         }
+        if schema['namespace'] is None:
+            del schema['namespace']

Review comment:
       Instead of adding and then deleting, we could just add it only if it's not None

##########
File path: pulsar-client-cpp/python/pulsar/schema/definition.py
##########
@@ -57,6 +57,8 @@ def _get_fields(cls, dct):
 
 class Record(with_metaclass(RecordMeta, object)):
 
+    _namespace = None

Review comment:
       One other thing is that we should add this to the Python schema docs 

##########
File path: pulsar-client-cpp/python/pulsar/schema/definition.py
##########
@@ -57,6 +57,8 @@ def _get_fields(cls, dct):
 
 class Record(with_metaclass(RecordMeta, object)):
 
+    _namespace = None

Review comment:
       I was wondering given that this is very specific to Avro, if we should name it as `_avro_namespace` to make it clear.




-- 
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 merged pull request #12175: [Python Schema] Support setting namespace for python schema

Posted by GitBox <gi...@apache.org>.
merlimat merged pull request #12175:
URL: https://github.com/apache/pulsar/pull/12175


   


-- 
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