You api could be greatly improved about aioamqp

[anti1869]

Hi guys,

+1 to not hiding AMQP wording, because it makes a lot easier to use this lib with AMQP reference guide and comparison with other implementations of the protocol, while lib's own docs are not available.

When I was needed to hide protocol details under abstraction layer, I just wrote a quick wrapper.

[dzen]

Agreed, and this is why this lib is still in version 0.xx: because the API is not clearly defined and not frozen. We first want to implement the specs and make sure it is efficient.

Don't hesitate to contribute; I think trying to improve the code and design together is the beauty of opensource.

As you may notice, maintaining an opensource library is very time-consuming and requires a lot of effort. Don't forget you're talking to humans, not to robots :))

[purpleP]

@dzen Yes, very consuming, I agree. That's what's kind of sad to me, so much effort is kind of wasted. Glad to hear, that you would allow contributions. I'll try to contribute. Actually, I'm rewriting parts of code right now. See what I can make myself =)

[dzen]

@purpleP try to not rewrite everything, since it will be hard to read
For instance, have a look on the enums.

Note: we can't hide the AMQP wording at all, since its very important to be sure RabbitMQ is behaving nicely with a low level lib.

[purpleP]

@dzen I don't understand how hiding implementation makes someone unsure about RabbitMQ behaviour. And by the way, rabbit isn't only implementation (just in case).

Second. I've started to play with the code. You can see current state in my fork.
Right now I have a question - where have got amqp 0.9.1 specs? I can't find any information about types encoding, frames etc. I think I found at least one bug in course of refactoring, but I can't be sure, because I can't find specs.

[purpleP]

@dzen I can see xml schema or something like that, that have description of frames etc, but not the types and serialization stuff.

[dzen]

Hello @purpleP,

I've got multiple points:

• I don't want this lib to hide all the AMQP specs / wording :
  For instance, using enums for exchange kinds prevent from using a new exchange kind provided from an extension.
  Exchange declare is the name of the RPC in the specs. I understand you're not happy with that.
  This is not a maxi high level library that makes your messages into a queue, with a magic class allowing you to unqueues them magically.
• Most of the specs are available in RabbitMQ website

I really feel you're angry, and sincerely I do not like your aggressiveness, but I think you're here to help.
We've been busy for month, and we know there are some bugs in the lib, but I'm sure we need to use some method to clean them up, and build a better lib.

Note: I did'nt find any commits in your branches, and I fear you already have a lot's of diff in your repository making it really hard to review.
We should write a plan, having goals to a 1.0 version and I probably must create a new github issue with this goal in mind having multiple users writing their will
and creating sub-issues, to ensure we all fix them.

[dzen]

@anti1869 yes it's my point :)

[purpleP]

@dzen I'm not angry, don't worry.
You haven't answered my question. I'll repeat it. Where can I find information about how different amqp types should be serialized?

I'll give an example of what I'm talking about and why I'm asking.

    _table_subitem_reader_map = {
        't': 'read_bit',
        'b': 'read_octet',
        'B': 'read_signed_octet',
        'U': 'read_signed_short',
        'u': 'read_short',
        'I': 'read_signed_long',
        'i': 'read_long',
    >>  'L': 'read_unsigned_long_long',
        'l': 'read_long_long',
        'f': 'read_float',
        'd': 'read_float',
        'D': 'read_decimal',
        's': 'read_shortstr',
        'S': 'read_longstr',
        'A': 'read_field_array',
        'T': 'read_timestamp',
        'F': 'read_table',
    }

You're using this to decide how to read table and array items, but there's no read_unsigned_long_long function. There is however read_signed_long_long function. Maybe you've meant to type that. I don't know and can't check that, because can't find this kind of info anywhere. I've tried to see pika's library implementation, but that's not a 100 % guarantee.

As for diffs. I have exactly one diff now basically. I've created new file types.py where I'm rewriting serialization/deserialization level. Check it out sometime around tomorrow. I think it will be ready by that time.

Quick question that's related to my first question. When writing tables

    def write_value(self, value):
        if isinstance(value, (bytes, str)):
            self.payload.write(b'S')
            self.write_longstr(value)
        elif isinstance(value, bool):
            self.payload.write(b't')
            self.write_bool(value)
        elif isinstance(value, dict):
            self.payload.write(b'F')
            self.write_table(value)
        elif isinstance(value, int):
            self.payload.write(b'I')
            self.write_long(value)
        else:
            raise Exception("type({}) unsupported".format(type(value)))

You don't have writers for all possible types. Is this the part of the spec? And if it's not wouldn't it be better to write more optimally in space? Like if the string is short enough that write shortstr type or if int is small enough then write short type etc.?

Oh, and It would probably be better to ask this upfront. why there is no 'write_array' function? Client never write arrays?

My plan right now is to add tests for decoding (I don't think you have any), probably rewrite tests for encoding. It could be made into single test for example.

def test_encoding_decoding(correct_binary, type, value):
    encoded = encode(type, value)
    assert encoded == correct_binary
    assert decode(type, encoded) == value

One other thing. After I'm done with encoding/decoding module it will be possible to memoize encoding and decoding of types and frames. Do you think this could be useful? I think it can, because typical amqp application uses limited set of different messages types. And because of that there would probably be a lot of identical headers and stuff like that.

[RemiCardona]

After I'm done with encoding/decoding module it will be possible to memoize encoding and decoding of types and frames. Do you think this could be useful?

I'm not against it, but ATM no-one (@dzen and I included) did any sort of profiling on the code base (other than 30,000ft in #70) and I'd rather avoid premature optimizations. Especially if the long-term maintenance cost is high.

Right now, the 2 areas I'd like to focus on are:

• protocol correctness/robustness (plenty of open issues/PRs already)
• user-facing APIs (including some of the issues you mentioned earlier, #90 is another huge issue)

Thanks for your suggestions (and patches!)

[purpleP]

@dzen memoization is done with functions.lru_cache decorator, so basically no maintenance cost.

I thought about that rewriting from the low-level to high level will bring fruits later, but It's easier to me to work that way. My workflow is something like:

1. Okay, how is this works? I don't understand a bit.
2. Okay, this is done first, than this, than this.
3. Hmm, strange code, why is this done that way? (Using bytesio in serialization in your case for example).
4. Oh, that's why.
5. This could be improved (less code, faster or memory usage or better API).

[martyanov]

I prefer the library to be as low-level as possible, it's easy to write high-level API over low-level one, but the opposite is not true. For example, another asynchronous RabbitMQ library aio-pika has API very similar to the one topic author suggested, queue-exchange binding looks like:

await queue.bind(exchange, routing_key)

It's pythonic, fun and dandy, but completely unusable, because it's not possible to create the binding without prepared queue instance and that's not always what you wanted. With low-level API that aioampq provides I can implement logic like this:

for queue in rabbitmq.queues:
    await self.declare_queue(**queue)

for exchange in rabbitmq.exchanges:
    await self.declare_exchange(**exchange)

for binding in rabbitmq.bindings:
    await self.bind_queue(**binding)

It's a snippet from my current project and I've implemented high level API over aioamqp's one, rabbitmq here is just a dictionary with attribute-like access.

So, if you gonna implement high-level API please leave the current one untouched.

Thanks! :)