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

Feature/improved structuring #97

Closed nklapste closed 5 years ago

nklapste commented 5 years ago

Another restructuring of graypy focusing on making the construction of handlers more straight forward. Along with improving testing coverage for sanity checking.

Note: Due to the changes in naming and separation of handlers these changes are backwards breaking.

Work in progress:

added a stub within the .travis.yml for adding automatic deployment to pypi

Features:

Detailed docstrings noting the usage of components
Typing noted in the docstrings
Configuration notes are removed from the README.rst duplicates the docstrings (maybe we should use read the docs or somesort?)
Improved testing by using a docker container running graylog to send test messages to
GELFHTTPHandler handler has been added
GELF encoding is now contained within BaseGELFHandler
TCP and TLS handlers are now separated to avoid confusion

codecov-io commented 5 years ago

Codecov Report

Merging #97 into master will increase coverage by 43.2%. The diff coverage is 98.43%.

@@            Coverage Diff             @@
##           master      #97      +/-   ##
==========================================
+ Coverage   51.48%   94.69%   +43.2%     
==========================================
  Files           3        3              
  Lines         202      226      +24     
==========================================
+ Hits          104      214     +110     
+ Misses         98       12      -86

Impacted Files	Coverage Δ
graypy/rabbitmq.py	`86.2% <100%> (+78.39%)`	:arrow_up:
graypy/__init__.py	`71.42% <100%> (-28.58%)`	:arrow_down:
graypy/handler.py	`98.75% <98.31%> (+28.52%)`	: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 cfe559a...e743677. Read the comment docs.

severb commented 5 years ago

This PR looks fantastic, is it ready for master? If so, please land and bump the version to 0.4.0.

nklapste commented 5 years ago

This PR looks fantastic, is it ready for master? If so, please land and bump the version to 0.4.0.

I would have to look over my work on this branch again, it has been a long time since I looked back at this branch. I should be able to re-review sometime this week.

I do believe some of the API for initializing some of these classes did in fact change. I would recommended bumping to a 1.X.X build if we merge and update.

nklapste commented 5 years ago

One thing to note was the the creation of TCP and TLS has been separated into different classes GELFTCPHandler and GELFTLSHandler . This was to remove the usage of optional arguments.

severb commented 5 years ago

You're right—we should bump the major component if it breaks backward compatibility.

nklapste commented 5 years ago

Everything, looks like how I left it. It should be ready to merge. I would re-run the Travis tests on the current build as well.

Other than that, I would recommend bumping to 1.X.X to denote api changes.

Also I would test the rabbitmq.py components manually. I was never able to make in-depth tests or setup an environment for this module.

nklapste commented 5 years ago

Seems that the tests for py3.4 are failing to package version conflicts. I'll look into it tonight.

nklapste commented 5 years ago

Should be resolved with locking the test requirements:

"pytest>=2.8.7,<4.0.0",
"pytest-cov<=2.6.0,<3.0.0",
"pylint>=1.9.3,<2.0.0",

nklapste commented 5 years ago

@severb everything should be good to go. Do you want me to merge then bump the version to 1.0.0 and tag?