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
105 stars 25 forks source link

Warning message about missing psycopg2, even when not using Postgres #27

Closed volcan01010 closed 4 years ago

volcan01010 commented 4 years ago

Summary

When using etlhelper in a new virtual environment without installing PostgreSQL dependencies, there are occasional error messages about missing psycopg2. These should be removed.

TODO: add steps to reproduce

Acceptance criteria

volcan01010 commented 4 years ago

I've found a way to reproduce this now:

  1. Create a new virtualenv and install etlhelper without any database drivers
  2. Run the following:
from etlhelper import DbParams
db = DbParams(dbtype='ORACLE', host='localhost', port=1, dbname='test', user='test')

This will create db, but also print the following:

The cxOracle drivers were not found. See setup guide for more information.

This message, and the psycopg2 one, are printed when you initialise a DbHelper for a database when the drivers are not installed. DbParams does this because it uses a DbHelper to validate the params when it is created. We are seeing the message in internal BGS projects because we often import a big list of DbParams for internal databases, which includes Oracle and PostgreSQL instances.

volcan01010 commented 4 years ago

I don't think that this is a bug - we do want to tell users how to install drivers if they are missing and we do want to all DbParams objects to be created in this case. However, we should replace the print statement with either a warnings.warn or a logger.warn message.

metazool commented 4 years ago

I don't think it is a bug either but it is a glitch. I ran into it during one of the Linked Data projects, with a script that dumps an export from Oracle straight to stdout, ended up installing psycopg2 in the CI pipeline even though i didn't need it, just in order to stop this warning message ending up in the output - it could go to stderr instead ?