cnzakii commented on code in PR #51:
URL: https://github.com/apache/dubbo-python/pull/51#discussion_r2322097087
##########
samples/llm/chat_pb2.py:
##########
@@ -1,13 +1,15 @@
-# -*- coding: utf-8 -*-
# Generated by the protocol buffer compiler. DO NOT EDIT!
Review Comment:
As the hint says, DO NOT EDIT!
##########
src/dubbo/classes.py:
##########
@@ -244,3 +246,22 @@ class ReadWriteStream(ReadStream, WriteStream, abc.ABC):
"""
pass
+
+
+class Codec(ABC):
+ def __init__(self, model_type: Optional[type[Any]] = None, **kwargs):
+ self.model_type = model_type
+
+ @abstractmethod
+ def encode(self, data: Any) -> bytes:
+ pass
+
+ @abstractmethod
+ def decode(self, data: bytes) -> Any:
+ pass
+
+
+class CodecHelper:
+ @staticmethod
+ def get_class():
+ return Codec
Review Comment:
Why is an additional `CodecHelper` wrapper needed?
##########
src/dubbo/client.py:
##########
@@ -91,116 +84,82 @@ def _initialize(self):
self._initialized = True
- @classmethod
- def _infer_types_from_interface(cls, interface: Callable) -> tuple:
- """
- Infer method name, parameter types, and return type from a callable.
- """
- try:
- type_hints = get_type_hints(interface)
- sig = inspect.signature(interface)
- method_name = interface.__name__
- params = list(sig.parameters.values())
-
- # skip 'self' for bound methods
- if params and params[0].name == "self":
- params = params[1:]
-
- param_types = [type_hints.get(p.name, Any) for p in params]
- return_type = type_hints.get("return", Any)
-
- return method_name, param_types, return_type
- except Exception:
- return interface.__name__, [Any], Any
-
def _create_rpc_callable(
self,
rpc_type: str,
- interface: Optional[Callable] = None,
- method_name: Optional[str] = None,
- params_types: Optional[List[Type]] = None,
- return_type: Optional[Type] = None,
+ method_name: str,
+ params_types: List[Type],
+ return_type: Type,
codec: Optional[str] = None,
request_serializer: Optional[SerializingFunction] = None,
response_deserializer: Optional[DeserializingFunction] = None,
- default_method_name: str = "rpc_call",
) -> RpcCallable:
"""
Create RPC callable with the specified type.
"""
- if interface is None and method_name is None:
- raise ValueError("Either 'interface' or 'method_name' must be
provided")
-
- # Start with explicit values
- m_name = method_name
- p_types = params_types
- r_type = return_type
-
- # Infer from interface if needed
- if interface:
- if p_types is None or r_type is None or m_name is None:
- inf_name, inf_params, inf_return =
self._infer_types_from_interface(
- interface
- )
- m_name = m_name or inf_name
- p_types = p_types or inf_params
- r_type = r_type or inf_return
-
- # Fallback to default
- m_name = m_name or default_method_name
-
+ print("2", params_types)
# Determine serializers
if request_serializer and response_deserializer:
req_ser = request_serializer
res_deser = response_deserializer
else:
- req_ser, res_deser =
DubboTransportService.create_serialization_functions(
- codec or "json", # fallback to json
- parameter_types=p_types,
- return_type=r_type,
+ req_ser, res_deser =
DubboSerializationService.create_serialization_functions(
+ codec or "json",
Review Comment:
JSON cannot simply be used as a fallback strategy. Unless you have a
complete decision mechanism, do not set it as a fallback (I already mentioned
this in the previous review...).
##########
src/dubbo/codec/json_codec/json_transport_base.py:
##########
@@ -0,0 +1,71 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from typing import Any, Callable, Protocol, Optional
+
+
+class JsonSerializerPlugin(Protocol):
+ """Protocol for JSON serialization plugins"""
+
+ def encode(self, obj: Any) -> bytes: ...
+ def decode(self, data: bytes) -> Any: ...
+ def can_handle(self, obj: Any) -> bool: ...
+
+
+class TypeHandlerPlugin(Protocol):
+ """Protocol for type-specific serialization"""
+
+ def can_serialize_type(self, obj: Any, obj_type: type) -> bool: ...
+ def serialize_to_dict(self, obj: Any) -> Any: ...
+
+
+class SimpleRegistry:
+ """Simplified registry using dict instead of complex
TypeProviderRegistry"""
+
+ def __init__(self):
+ # Simple dict mapping: type -> handler function
+ self.type_handlers: dict[type, Callable[..., Any]] = {}
+ self.plugins: list[TypeHandlerPlugin] = []
+
+ def register_type_handler(self, obj_type: type, handler: Callable[...,
Any]) -> None:
+ """Register a simple type handler function"""
+ self.type_handlers[obj_type] = handler
+
+ def register_plugin(self, plugin: TypeHandlerPlugin) -> None:
+ """Register a plugin"""
+ self.plugins.append(plugin)
+
+ def get_handler(self, obj: Any) -> Optional[Callable[..., Any]]:
+ """Get handler for object - check dict first, then plugins"""
+ obj_type = type(obj)
+ if obj_type in self.type_handlers:
+ return self.type_handlers[obj_type]
+
+ for plugin in self.plugins:
+ if plugin.can_serialize_type(obj, obj_type):
+ return plugin.serialize_to_dict
+ return None
Review Comment:
Same old issue... Please integrate with Dubbo’s extension...
##########
src/dubbo/client.py:
##########
@@ -82,82 +84,82 @@ def _initialize(self):
self._initialized = True
- def unary(
+ def _create_rpc_callable(
self,
+ rpc_type: str,
method_name: str,
+ params_types: List[Type],
+ return_type: Type,
+ codec: Optional[str] = None,
request_serializer: Optional[SerializingFunction] = None,
response_deserializer: Optional[DeserializingFunction] = None,
) -> RpcCallable:
- return self._callable(
- MethodDescriptor(
- method_name=method_name,
- arg_serialization=(request_serializer, None),
- return_serialization=(None, response_deserializer),
- rpc_type=RpcTypes.UNARY.value,
+ """
+ Create RPC callable with the specified type.
+ """
+ print("2", params_types)
Review Comment:
What is this?
##########
src/dubbo/proxy/handlers.py:
##########
@@ -27,57 +28,96 @@
class RpcMethodHandler:
- """
- Rpc method handler
- """
-
__slots__ = ["_method_descriptor"]
def __init__(self, method_descriptor: MethodDescriptor):
- """
- Initialize the RpcMethodHandler
- :param method_descriptor: the method descriptor.
- :type method_descriptor: MethodDescriptor
- """
self._method_descriptor = method_descriptor
@property
def method_descriptor(self) -> MethodDescriptor:
- """
- Get the method descriptor
- :return: the method descriptor
- :rtype: MethodDescriptor
- """
return self._method_descriptor
+ @staticmethod
+ def get_codec(**kwargs) -> tuple:
+ return DubboTransportService.create_serialization_functions(**kwargs)
+
+ @classmethod
+ def _infer_types_from_method(cls, method: Callable) -> tuple:
+ try:
+ type_hints = get_type_hints(method)
+ sig = inspect.signature(method)
+ method_name = method.__name__
+ params = list(sig.parameters.values())
+ if params and params[0].name == "self":
Review Comment:
This issue still hasn’t been resolved.
##########
src/dubbo/codec/protobuf_codec/protobuf_codec_handler.py:
##########
@@ -0,0 +1,305 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from typing import Any, Type, Protocol, Optional
+from abc import ABC, abstractmethod
+import json
+from dataclasses import dataclass
+
+# Betterproto imports
Review Comment:
This issue still hasn’t been resolved.
##########
src/dubbo/codec/json_codec/json_transport_codec.py:
##########
@@ -0,0 +1,231 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import json
+from dataclasses import asdict, is_dataclass
+from datetime import date, datetime, time
+from decimal import Decimal
+from enum import Enum
+from pathlib import Path
+from typing import Any, Union
+from uuid import UUID
+from typing import Optional
+
+
+class StandardJsonPlugin:
+ """Standard library JSON codec"""
+
+ def encode(self, obj: Any) -> bytes:
+ return json.dumps(obj, ensure_ascii=False, separators=(",",
":")).encode("utf-8")
+
+ def decode(self, data: bytes) -> Any:
+ return json.loads(data.decode("utf-8"))
+
+ def can_handle(self, obj: Any) -> bool:
+ return True
+
+
+class OrJsonPlugin:
+ """orjson Codec independent implementation"""
+
+ def __init__(self):
+ try:
+ import orjson
+
+ self.orjson = orjson
+ self.available = True
+ except ImportError:
+ self.available = False
+
+ def encode(self, obj: Any) -> bytes:
+ if not self.available:
+ raise ImportError("orjson not available")
+ return self.orjson.dumps(obj, default=self._default_handler)
+
+ def decode(self, data: bytes) -> Any:
+ if not self.available:
+ raise ImportError("orjson not available")
+ return self.orjson.loads(data)
+
+ def can_handle(self, obj: Any) -> bool:
+ return self.available
+
+ def _default_handler(self, obj):
+ """Handle types not supported natively by orjson"""
+ if isinstance(obj, datetime):
+ return {"__datetime__": obj.isoformat(), "__timezone__":
str(obj.tzinfo) if obj.tzinfo else None}
+ elif isinstance(obj, date):
+ return {"__date__": obj.isoformat()}
+ elif isinstance(obj, time):
+ return {"__time__": obj.isoformat()}
+ elif isinstance(obj, Decimal):
+ return {"__decimal__": str(obj)}
+ elif isinstance(obj, set):
+ return {"__set__": list(obj)}
+ elif isinstance(obj, frozenset):
+ return {"__frozenset__": list(obj)}
+ elif isinstance(obj, UUID):
+ return {"__uuid__": str(obj)}
+ elif isinstance(obj, Path):
+ return {"__path__": str(obj)}
+ return {"__fallback__": str(obj), "__type__": type(obj).__name__}
+
+
+class UJsonPlugin:
+ """ujson plugin implementation"""
+
+ def __init__(self):
+ try:
+ import ujson
+
+ self.ujson = ujson
+ self.available = True
+ except ImportError:
+ self.available = False
+
+ def encode(self, obj: Any) -> bytes:
+ if not self.available:
+ raise ImportError("ujson not available")
+ return self.ujson.dumps(obj, ensure_ascii=False,
default=self._default_handler).encode("utf-8")
+
+ def decode(self, data: bytes) -> Any:
+ if not self.available:
+ raise ImportError("ujson not available")
+ return self.ujson.loads(data.decode("utf-8"))
+
+ def can_handle(self, obj: Any) -> bool:
+ return self.available
+
+ def _default_handler(self, obj):
+ """Handle types not supported natively by ujson"""
+ if isinstance(obj, datetime):
+ return {"__datetime__": obj.isoformat(), "__timezone__":
str(obj.tzinfo) if obj.tzinfo else None}
+ elif isinstance(obj, date):
+ return {"__date__": obj.isoformat()}
+ elif isinstance(obj, time):
+ return {"__time__": obj.isoformat()}
+ elif isinstance(obj, Decimal):
+ return {"__decimal__": str(obj)}
+ elif isinstance(obj, set):
+ return {"__set__": list(obj)}
+ elif isinstance(obj, frozenset):
+ return {"__frozenset__": list(obj)}
+ elif isinstance(obj, UUID):
+ return {"__uuid__": str(obj)}
+ elif isinstance(obj, Path):
+ return {"__path__": str(obj)}
+ return {"__fallback__": str(obj), "__type__": type(obj).__name__}
+
Review Comment:
Please refer to the implementation in `compression`, which is the simplest
extension mechanism, including both interface definition and implementation.
--
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: notifications-unsubscr...@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For additional commands, e-mail: notifications-h...@dubbo.apache.org
