ably / ably-python

Python client library SDK for Ably realtime messaging service
https://ably.com/download
Apache License 2.0
49 stars 24 forks source link

Publishing data with datetime throws TypeError #527

Open Andrioden opened 1 year ago

Andrioden commented 1 year ago

Here should be a minimal script to reproduce it

ably_test.py

import asyncio
from datetime import datetime

from ably import AblyRest

async def main():
    _client = AblyRest("yourkey")
    channel = _client.channels.get("global")
    await channel.publish("test", {"date": datetime.now()})

if __name__ == "__main__":
    asyncio.run(main())

Run with python .\ably_test.py, throws

Traceback (most recent call last):
  File "C:\Repos\untitledgame\ably_test.py", line 28, in <module>
    asyncio.run(main())
  File "C:\Bin\Python311\Lib\asyncio\runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "C:\Bin\Python311\Lib\asyncio\runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Bin\Python311\Lib\asyncio\base_events.py", line 653, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "C:\Repos\untitledgame\ably_test.py", line 21, in main
    await channel.publish("test", {"kek": "lol", "date": datetime.now()})
  File "C:\Users\andre\.virtualenvs\untitledgame-INXEEyOC\Lib\site-packages\ably\rest\channel.py", line 132, in publish
    return await self._publish(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\andre\.virtualenvs\untitledgame-INXEEyOC\Lib\site-packages\ably\rest\channel.py", line 104, in publish_name_data
    return await self.publish_messages(messages, timeout=timeout)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\andre\.virtualenvs\untitledgame-INXEEyOC\Lib\site-packages\ably\rest\channel.py", line 89, in publish_messages
    request_body = self.__publish_request_body(messages)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\andre\.virtualenvs\untitledgame-INXEEyOC\Lib\site-packages\ably\rest\channel.py", line 70, in __publish_request_body
    request_body = [
                   ^
  File "C:\Users\andre\.virtualenvs\untitledgame-INXEEyOC\Lib\site-packages\ably\rest\channel.py", line 71, in <listcomp>
    message.as_dict(binary=self.ably.options.use_binary_protocol)
  File "C:\Users\andre\.virtualenvs\untitledgame-INXEEyOC\Lib\site-packages\ably\types\message.py", line 139, in as_dict
    data = json.dumps(data)
           ^^^^^^^^^^^^^^^^
  File "C:\Bin\Python311\Lib\json\__init__.py", line 231, in dumps
    return _default_encoder.encode(obj)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Bin\Python311\Lib\json\encoder.py", line 200, in encode
    chunks = self.iterencode(o, _one_shot=True)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Bin\Python311\Lib\json\encoder.py", line 258, in iterencode
    return _iterencode(o, 0)
           ^^^^^^^^^^^^^^^^^
  File "C:\Bin\Python311\Lib\json\encoder.py", line 180, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type datetime is not JSON serializable

Environment:

- Windows: 11
- Python: 3.11.3
- Ably: 2.0.1

┆Issue is synchronized with this Jira Task by Unito

sync-by-unito[bot] commented 1 year ago

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-3841

sacOO7 commented 1 year ago

Hi, @Andrioden we will look into it. Meanwhile, can you try serializing it externally, sending it as an int timestamp instead wdyt?

sacOO7 commented 1 year ago

Link to reference DateTime serialization implementation - https://stackoverflow.com/a/22238613

sacOO7 commented 1 year ago

https://pynative.com/python-serialize-datetime-into-json

Andrioden commented 1 year ago

@sacOO7 - Thanks for the quick response. Changing the data contract is not a good option as i use it for api http server->client data transfer as well. I also prefer str dates over int dates as part of the contracts as they are more readable/debuggable. Instead I went with the following workaround as hinted by you.


EventPublisher - My server helper class

class EventPublisher:
    @staticmethod
    def _json_serializer(obj: dict) -> str:
        """JSON serializer for objects not serializable by default json code"""
        if isinstance(obj, (datetime, date)):
            return obj.isoformat()
        raise TypeError("Type %s not serializable" % type(obj))

    async def publish_to_all_async(self, name: str, data: dict) -> None:
        _client = AblyRest(config.ably_server_api_key)
        channel = _client.channels.get("global")
        await channel.publish(name, json.dumps(data, default=self._json_serializer))
        await _client.close()

.isoformat() works really well, also if datetimes have timezone info. I also tested this. Example: datetime.now(tz=ZoneInfo("America/New_York"))


EventSubscriber - My client web/javascript helper class

export class EventSubscriber {
    /**
     * @param {string} apiKey
     */
    static async init(apiKey) {
        const ably = new Ably.Realtime.Promise(apiKey)
        await ably.connection.once("connected")
        console.log("Event subscriber connected!")

        await ably.channels
            .get("global")
            .subscribe((message) => this.processMessage(message))
    }

    /**
     * @param {Ably.Types.Message} message
     */
    static processMessage(message) {
        const dataJson = JSON.parse(message.data)
        // Do stuff with data...
    }
}

Any permanent solution that you implement should:

sacOO7 commented 1 year ago

@sacOO7 - Thanks for the quick response. Changing the data contract is not a good option as i use it for api http server->client data transfer as well. Instead I went with the following workaround as hinted by you.

EventPublisher - My server helper class

class EventPublisher:
    @staticmethod
    def _json_serializer(obj: dict) -> str:
        """JSON serializer for objects not serializable by default json code"""
        if isinstance(obj, (datetime, date)):
            return obj.isoformat()
        raise TypeError("Type %s not serializable" % type(obj))

    async def publish_to_all_async(self, name: str, data: dict) -> None:
        _client = AblyRest(config.ably_server_api_key)
        channel = _client.channels.get("global")
        await channel.publish(name, json.dumps(data, default=self._json_serializer))
        await _client.close()

.isoformat() works really well, also if datetimes have timezone info. I also tested this. Example: datetime.now(tz=ZoneInfo("America/New_York"))

EventSubscriber - My client web/javascript helper class

export class EventSubscriber {
    /**
     * @param {string} apiKey
     */
    static async init(apiKey) {
        const ably = new Ably.Realtime.Promise(apiKey)
        await ably.connection.once("connected")
        console.log("Event subscriber connected!")

        await ably.channels
            .get("global")
            .subscribe((message) => this.processMessage(message))
    }

    /**
     * @param {Ably.Types.Message} message
     */
    static processMessage(message) {
        const dataJson = JSON.parse(message.data)
    }
}

Any permanent solution that you implement should:

  • remove the need to json serialize / deserialize the data,
  • and handle timezone aware and non-aware datetimes published from the python server, by preserving the timezone data between server-client.

Yeah, we will try to implement the same 👍

sacOO7 commented 1 year ago

This is going to take a good amount of time looking at the wide use of json.dumps and json.loads at multiple places. Need to refactor it with one custom JSON serializer and deserializer. Also, need to refactor msgpack use at multiple places. Still, @Andrioden we will create PR to fix this soon. Do star the repo if not already : )