BritishGeologicalSurvey / etlhelper

ETL Helper is a Python ETL library to simplify data transfer into and out of databases.
https://britishgeologicalsurvey.github.io/etlhelper/
GNU Lesser General Public License v3.0
100 stars 25 forks source link

125 refactor logging to add log to stderr function #171

Closed leorudczenko closed 1 year ago

leorudczenko commented 1 year ago

This merge request adds the function log_to_console, which allows users to enable and modify the etlhelper logging handler, as specified by issue #125.

UPDATE:

To fix the issues described in the comments, 2 extra changes have been made:

Closes #125

leorudczenko commented 1 year ago

The GitHub workflow successfully ran the tests, however that does not include all of the tests. The test file test/integration/etl/test_etl_logging.py needs to be updated to reflect the log changes.

leorudczenko commented 1 year ago

The test file test/integration/etl/test_etl_logging.py has now been updated to reflect the log changes. I have run these tests locally and confirmed they work as expected.

leorudczenko commented 1 year ago

There is 1 more test that is currently failing in test/integration/etl/test_abort.py, test_abort_etlhelper_threads[do_executemany_etl-executemany]. Once this has been fixed then this will be ready to merge

leorudczenko commented 1 year ago

The error seems to come from the fact that many files are still initialising an etlhelper logger using logger = logging.getLogger("etlhelper"). But since the __init__ file no longer creates the logger by default, the calls to get the logger in other files are actually create a standard new logger.

When adding logging.getLogger("etlhelper").addHandler(logging.NullHandler()), to the etlhelper/__init__.py file, running the test test_abort_etlhelper_threads alone passes. However, when running all the tests together, test_abort_etlhelper_threads fails again.

leorudczenko commented 1 year ago

test_abort_etlhelper_threads was still failing because test/integration/etl/test_etl_logging.py enabled the logging for etlhelper, but test_abort_etlhelper_threads requires the logging to be disabled.

To fix this, I've created a pytest fixture which yields an enabled etlhelper logger, and then sets the handler back to NullHandler after completion.

With this change, all of the tests are now passing as expected.

volcan01010 commented 1 year ago

Tested locally during pairing session - all good!