cmanaha / python-elasticsearch-logger

Python Elasticsearch handler for the standard python logging framework
Other
232 stars 116 forks source link

Schedule flush later #17

Closed KonstantinSchubert closed 7 years ago

KonstantinSchubert commented 7 years ago

( This builds upon my previous pull request that adds AWS elasticsearch logging, so I guess it should be reviewed afterwards. )

Instead of scheduling the flush "recursively", with this pull request, a flush will be scheduled under two conditions:

  1. No flush is scheduled yet, and
  2. there is actually something in the buffer that needs to be flushed.

This has two advantages:

  1. Flushes only happen if something is in the buffer.
  2. If the process is forked after the __init__ is called, the logging handler is copied into each process. Before, it would flush the buffer only in the original process and not log in any forked processes. Now, it will be logging in all processes. I noticed that I needed this for example for django_q.
codecov-io commented 7 years ago

Codecov Report

Merging #17 into master will decrease coverage by 6.37%. The diff coverage is 48%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #17      +/-   ##
==========================================
- Coverage   87.39%   81.02%   -6.38%     
==========================================
  Files           1        1              
  Lines         119      137      +18     
  Branches       14       18       +4     
==========================================
+ Hits          104      111       +7     
- Misses         13       22       +9     
- Partials        2        4       +2
Impacted Files Coverage Δ
cmreslogging/handlers.py 81.02% <48%> (-6.38%) :arrow_down:

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 f047106...a003df6. Read the comment docs.

cmanaha commented 7 years ago

Thanks for all this changes @KonstantinSchubert !