mosquito / aio-pika

AMQP 0.9 client designed for asyncio and humans.
https://aio-pika.readthedocs.org/
Apache License 2.0
1.23k stars 187 forks source link

Timestamp not accepting any int #210

Open Artimi opened 5 years ago

Artimi commented 5 years ago

Hi,

I'm updating our system to aio_pika>=5.4.0 and I see that you changed the Message.timestamp type to datetime.datetime

[HIGH] Message.timestamp property is now datetime.datetime

In our system, we are using as timestamp int 64-bit which store a number of nanoseconds since the epoch. It does not oppose RabbitMQ specification because timestamp there is also 64-bit int. Wouldn't it be easier to let the type of timestamp on the user instead of forcing a type there? Do you have any idea how could I use the new aio_pika with nanoseconds since epoch?

mosquito commented 5 years ago

It's should be closed with pamqp release.

Artimi commented 5 years ago

Thanks for the fix. However, I have an issue with this: I wanted to send nanoseconds since epoch (e.g. 1559224445019561728) because we are using nanoseconds everywhere (we come from a banking sector so we appreciate precise time). If we would send it as seconds since epoch we could not use the timestamp to check messaging lag (1 second precision is too much). Do you think this will be possible again?

Artimi commented 5 years ago

I tried to work around this by sending the timestamp in headers. It works but it is complicated because I have to update every producer in our infrastructure before I can update aio_pika. Could we maybe use some flag that would not interpret timestamp? It is a 64-bit integer so it is quite waste to store only seconds since epoch there. Even milliseconds or better microseconds would be helpful but nothing like this is now possible.

Artimi commented 5 years ago

Ok, so I decided to bypass every check in aio_pika and pamqp and it seems to work. It is just a hack but I have no other option.

import struct

import aio_pika
import pamqp

def encode_timestamp(value):
    """Encode an int to RabbitMQ timestamp
    :param value: Value to encode
    :type value: datetime.datetime, time.struct_time, integer
    :rtype: bytes
    """
    return struct.pack('>Q', value)

def decode_timestamp(value):
    """Decode a timestamp value to int.
    :param bytes value: Value to decode
    :return tuple: bytes used, struct_time
    :raises: ValueError
    """
    try:
        value = struct.unpack('>Q', value[0:8])
        return 8, value[0]
    except TypeError:
        raise ValueError('Could not unpack data')

pamqp.encode.timestamp = encode_timestamp
pamqp.decode.timestamp = decode_timestamp

def identity(value):
    return value

aio_pika.message.encode_timestamp = identity
aio_pika.message.decode_timestamp = identity
mosquito commented 2 years ago

@Artimi so much has been changed in aio-pika, aiormq and pamqp, do you think it's still relevant?

Artimi commented 2 years ago

I haven't tried it but from the brief look at the pamqp code it still seems that you cannot pass any int as a timestamp. Thus it is still not according to AMQP specification but since we've found a workaround (which we have to change every now and then but whatever). If you decide that you don't want to support this we'll understand.