-
-
Notifications
You must be signed in to change notification settings - Fork 624
feat: support to msgpack ext type #1517
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
base: main
Are you sure you want to change the base?
Conversation
|
The coverage action is failing for the following reason, but it doesn't seem to be caused by the changes in the PR. [2025-11-02T07:35:13.570Z] ['error'] There was an error running the uploader: Error uploading to [https://codecov.io:](https://codecov.io/) Error: There was an error fetching the storage URL during POST: 429 - {"message":"Rate limit reached. Please upload with the Codecov repository upload token to resolve issue. Expected time to availability: 685s."} |
|
Looks good, thanks! But how do you intend these new arguments will be passed when used in a complete application? Do you have any example code that shows how the integration works? |
I’m not sure I fully understand exactly what you need. Do you mean a sample code like the one below? from __future__ import annotations
from typing import TYPE_CHECKING, Any, override
from uuid import UUID
import msgpack
import socketio
from socketio.async_manager import AsyncManager
from socketio.msgpack_packet import MsgPackPacket as SocketIOMsgPackPacket
if TYPE_CHECKING:
from collections.abc import Buffer, Mapping
USERS: Mapping[UUID, Any] = {}
class MsgPackPacket(SocketIOMsgPackPacket):
@override
def encode(self) -> Any:
return msgpack.dumps(self._to_dict(), default=default_encode)
@override
def decode(self, encoded_packet: Buffer) -> None:
decoded = msgpack.loads(encoded_packet, ext_hook=ext_hook)
self.packet_type = decoded["type"]
self.data = decoded.get("data")
self.id = decoded.get("id")
self.namespace = decoded["nsp"]
def default_encode(value: Any) -> Any:
match value:
case UUID():
return msgpack.ExtType(0, value.bytes)
case _:
return value
def ext_hook(code: int, data: bytes) -> Any:
match code:
case 0:
return UUID(bytes=data)
case _:
error_msg = f"Unknown msgpack ext code: {code}"
raise ValueError(error_msg)
sio = socketio.AsyncServer(
cors_allowed_origins=[],
async_mode="asgi",
always_connect=True,
client_manager=AsyncManager(),
serializer=MsgPackPacket,
)
@sio.on("user-list")
async def user_list() -> None:
output = {"user_ids": list(USERS)}
await sio.emit("user-list", output)
I can’t share the code here, but we use socketio when the JS frontend and Python backend exchange messages, and we pass the uuid value using ext_type. The code above is a simplified version of that backend code. |
|
Ah — after writing the sample code I realized that because this class is being passed, it's not possible to provide arguments to its |
I think if the user uses |
|
Partial works, but I don't see it as a good solution. If anything it would make every packet creation slower. I have to think about this. I think passing these new arguments in the constructor of the packet is not the most usable solution. These have to be configured elsewhere and globally. |
|
I thought |
Shall I proceed with this task? |
|
It looks like if json is not None:
self.packet_class.json = json |
|
I've been thinking about this, and I'd like to have a simpler interface. For example: serializer=MsgPackPacket # use with default options
serializer=MsgPackPacket.configure(default=my_default, ext_hook=my_ext_hook) # custom configurationThis makes the configuration explicit, and visible to IDEs and type checkers, versus the Unfortunately implementing my the above pattern is not trivial, as it requires generating new classes on the fly. I'll see if I can modify this PR to do it over the next few days. |
Are you considering it as a class method of the |
|
I'm thinking the |
I implemented it roughly as a class method. Would this direction work? |
|
@phi-friday I appreciate you trying to implement what I'm saying, but I don't want you to waste time before I really figure out how I want this to work. I just wanted to provide you an update and to let you know that I needed to think some more about this. |
This change adds two new parameters,
dumps_defaultandext_hook, to enable support for msgpack ext type.With these arguments,
MsgPackPacketcan encode and decode values using msgpack ext type.