Wrong calculating of chunk size

[mastak]

=ERROR REPORT==== 16-Feb-2016::02:02:18 ===
Error on AMQP connection <0.25444.39> (172.17.0.1:56236 -> 172.17.0.19:5672, vhost: 'sync', user: 'some_producer', state: running), channel 1:
operation none caused a connection exception frame_error: "type 3, all octets = <<>>: {frame_too_large,131184,131064}"

I process the text data from external service, and sometimes it can contain some data like:

... \u2014 signal \u2014 medium ...

So after encode this string is increasing.

[dzen]

Hello @mastak,

I think this should probably be done in the encoder, to abstract the type from parent code ?

[mastak]

Hello @dzen,

To tell the truth, I do not quite understand what you mean...)

Some thing like that?

encoder = amqp_frame.AmqpEncoder()
encoder.payload.write(payload)
for encoder_chunk in encoder.chunks:
    content_frame = amqp_frame.AmqpRequest(
        self.protocol.writer, amqp_constants.TYPE_BODY, self.channel_id)
    content_frame.declare_class(amqp_constants.CLASS_BASIC)
    content_frame.write_frame(encoder_chunk)

According to the source code, encoder - it is parameter for write_frame. Encoding of payload data should probably be done before splitting into chunks, and if it (encoding and splitting) would be done in encoder, - it will be contain not one, but multiple items data for write_frame,

[dzen]

I thing you get the whole picture: we have to delegate the request to another class. This class should handle how we build a request for a chunk'd payload and deal with payload's encoding

What if we get a PayloadEncoder ? (note: this is a pseudo code, of course)

encoder_frame = PayloadEncoder()
encoder_frame.write_payload(payload, chunk_size=self.protocol.server_frame_max)

self.protocol.writer(encoder_frame.to_request_frame())

[mastak]

Ok, I will try to make it ;)

[foolswood]

Bump?

[RemiCardona]

So the fix looks good, but I'm just tempted to throw out the str support altogether. AMQP message body is clearly bytes. This would leave callers ultimately responsible for string encoding.

Would anyone object to the change? It would simply move the .encode() call higher up in the chain.

Cheers

[dzen]

Is this still the case since we just moved to pamqp ?

[RemiCardona]

That was fixed a while back in 09ec24d8 and str support was deprecated in 11754011 (included in 0.11.0). Now is probably a good time to remove str support for real.