graypy icon indicating copy to clipboard operation
graypy copied to clipboard

Updating to amqp version 2.4.2

Open bakkerd opened this issue 5 years ago • 7 comments

As amqplib is quite old (https://pypi.org/project/amqplib/#history) and I had some issues with reconnecting, I propose to update it to amqp, which has the same interface, but is maintained and widely used.

bakkerd avatar Sep 11 '19 19:09 bakkerd

Would it be possible to add some instrumentation testing for the GELFRabbitHandler to validate the implementation and usage of this library?

nklapste avatar Sep 16 '19 23:09 nklapste

It would be an integration test, using an instance of rabbitmq. They are not in place, so it is not trivial to implement, which is why I did not include tests yet. I see that you did add some for direct graylog integration testing, so I will try re-using those.

bakkerd avatar Sep 21 '19 11:09 bakkerd

Doing a integration test would be preferred. As it also give a implicit example on how to setup graypy -> rabbitmq -> graylog.

Good luck with working off of that past branch. I sadly was struggling with setting up a rabbitmq -> graylog environment with docker that was performant and effective for testing.

nklapste avatar Sep 25 '19 00:09 nklapste

Codecov Report

Merging #119 into master will increase coverage by 1.47%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #119      +/-   ##
==========================================
+ Coverage   95.97%   97.44%   +1.47%     
==========================================
  Files           3        3              
  Lines         273      274       +1     
==========================================
+ Hits          262      267       +5     
+ Misses         11        7       -4
Impacted Files Coverage Δ
graypy/rabbitmq.py 93.22% <100%> (+7.01%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d3d1d85...4d92ac6. Read the comment docs.

codecov-io avatar Mar 13 '20 16:03 codecov-io

@martinvy any updates on this PR? This seems to fix an issue I've been having with graypy/amqp.

The amqplib module is so old that is requires you to stick to a version of setuptools no newer than 59.8.0 in order to install graypy with the amqp extra. Otherwise after doing an install of graypy will get you this issue:

>>> import graypy 
>>> graypy.GELFRabbitHandler()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: module 'graypy' has no attribute 'GELFRabbitHandler'

It would be fantastic if we could get a new graypy release in pypi with this fix.

cyber-conor avatar Jun 21 '22 19:06 cyber-conor

@cyber-conor Good question, I think this pull request is good to go. At the time nobody seemed to work on this project anymore, but I see some recent updates in the last months. @nklapste, could you have a look at this, and if you have the “authority” merge this if you are content?

EDIT: @nklapste I even added the integration test you requested😉

bakkerd avatar Jun 21 '22 19:06 bakkerd

Would love to see this merged! 💯 I've been using @bakkerd 's fork and I have not come across any issues!

billsioros avatar Aug 28 '23 19:08 billsioros