nameko / nameko-amqp-retry

Other
24 stars 4 forks source link

Support Nameko 2.11.0 and v3 prerelease #26

Closed mattbennett closed 5 years ago

mattbennett commented 5 years ago

Changes:

Delgating to the Publisher utility means we can run against multiple versions of Nameko and whatever versions of Kombu that it chooses.

Additional changes for compatibility with the 3.x prerelease (3a10df6):

cblegare commented 5 years ago

Thank you for this.

As discussed in #25, I still have issues with some tests. I'll post examples here in case someone reproduce it

This one happens randomly on any version

____ TestMessaging.test_multiple_queues_with_same_exchange_and_routing_key _____
<snip>
>       assert backoff_queue['messages'] == 0
E       KeyError: 'messages'

This one only happened in py27-oldest-lib

__________________________ TestPublisher.test_routing __________________________
<snip>
>       assert backoff_queue['messages'] == delays.count(delay)
E       assert 0 == 1
E        +  where 1 = <built-in method count of list object at 0x7f7ced4f00e0>(10000)
E        +    where <built-in method count of list object at 0x7f7ced4f00e0> = [10000, 20000, 20000, 30000, 30000, 30000].count

test/test_retry.py:75: AssertionError
mattbennett commented 5 years ago

I can reliably reproduce the KeyError: 'messages' on RabbitMQ 3.6.7 and above. That release introduced the "distributed management plugin" which caused problems with the Nameko test suite too. From that release onwards changes made through the REST API are only eventually consistent. I worked around it in the Nameko testsuite by querying the state of the queues using AMQP instead. I can do that here too I think.

Is your other error consistent or sporadic?

cblegare commented 5 years ago

Sporadic (roughly 60% repro). This would be consistent with plugin change you mentionned

mattbennett commented 5 years ago

Great. I think all of these errors are explained by this change in the HTTP API. Inspecting the queues with AMQP seems to work :)

cblegare commented 5 years ago

Good!

Just saying, would it be a good idea to make the rabbit_manager use both http and amqp?

Thank you a lot for your work.

mattbennett commented 5 years ago

@cblegare rabbit_manager is just a thin wrapper around (some parts of) the REST API. I'd rather keep it simple than make just the queue inspection part of it use AMQP.

My colleague @kooba pointed out that this is branch is still not compatible with the pre-release versions of Nameko (3.x), so I will bundle that into this branch

mattbennett commented 5 years ago

The prerelease tests here will fail until the changes in https://github.com/nameko/nameko/pull/592 are released

cblegare commented 5 years ago

@mattbennett Thank you! I'll look into nameko 3 too