severb / graypy

Python logging handler for Graylog that sends messages in GELF (Graylog Extended Log Format).

https://www.graylog.org/

BSD 3-Clause "New" or "Revised" License

258 stars 90 forks source link

Updating to amqp version 2.4.2 #119

Open bakkerd opened 5 years ago

bakkerd commented 5 years ago

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.

nklapste commented 5 years ago

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

bakkerd commented 4 years ago

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.

nklapste commented 4 years ago

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.

codecov-io commented 4 years ago

Codecov Report

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

@@            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.

cyber-conor commented 2 years ago

@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.

bakkerd commented 2 years ago

@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😉

billsioros commented 1 year ago

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