Timescale backend teething troubles, take 2 (#243)

* add support for ssl connections to the ql db init container.

* log pg conn data and test reading from env.

* cater for new columns when exporting data from crate.

* modularise env config and make it unit testable.

* refactor tests to avoid nasty env var side-effects.

* keep code climate happy.

* implement pr feedback.

src/translators/factory.py

@@ -1,9 +1,8 @@
 import logging
-import os
-import yaml
 
 from translators.crate import CrateTranslatorInstance
 from translators.timescale import postgres_translator_instance
+from utils.cfgreader import YamlReader
 from utils.jsondict import maybe_string_match
 
 
@@ -14,23 +13,13 @@
 
 
 def log():
-    logging.basicConfig(level=logging.INFO)
     return logging.getLogger(__name__)
 
 
-def read_config() -> dict:
-    path = os.environ.get(QL_CONFIG_ENV_VAR)
-    if path:
-        log().info(f"using config file: {path}")
-        file = open(path)
-        return yaml.safe_load(file)
-
-    log().info(f"no config file specified, using defaults.")
-    return {}
-
-
 def translator_for(fiware_service: str):
-    config = read_config()
+    reader = YamlReader(log=log().info)
+    config = reader.from_env_file(QL_CONFIG_ENV_VAR, defaults={})
+
     backend = maybe_string_match(config, 'tenants', fiware_service, 'backend')\
         or maybe_string_match(config, 'default-backend')
     backend = backend.strip().lower() if backend is not None else ''

src/translators/tests/run_tests.sh

@@ -15,9 +15,21 @@ docker-compose pull timescale
 docker-compose up -d
 sleep 20
 
+
+
 cd ../../../
+
+# Set Postgres port to same value as in docker-compose.yml
+export POSTGRES_PORT='54320'
+# Set test QL config file
+export QL_CONFIG='src/translators/tests/ql-config.yml'
+
 pytest src/translators/ --cov-report= --cov-config=.coveragerc --cov=src/
 r=$?
+
+unset POSTGRES_PORT
+unset QL_CONFIG
+
 cd -
 
 docker-compose down -v

src/translators/tests/test_pg_conn_data.py

@@ -0,0 +1,58 @@
+import pytest
+
+from translators.timescale import PostgresConnectionData
+
+
+def assert_conn_param(getter, env_var_name, env_var_value,
+                      expected_param_value=None):
+    conn_data = PostgresConnectionData()
+    default_value = getter(conn_data)
+    conn_data.read_env(env={env_var_name: env_var_value})
+
+    if expected_param_value:
+        assert getter(conn_data) == expected_param_value
+    else:
+        assert getter(conn_data) == default_value
+
+
+def get_host(c: PostgresConnectionData): return c.host
+
+
+def get_port(c: PostgresConnectionData): return c.port
+
+
+def get_ssl(c: PostgresConnectionData): return c.use_ssl
+
+
+def get_db_name(c: PostgresConnectionData): return c.db_name
+
+
+def get_db_user(c: PostgresConnectionData): return c.db_user
+
+
+def get_db_pass(c: PostgresConnectionData): return c.db_pass
+
+
+@pytest.mark.parametrize('getter, env_var_name', [
+    (get_host, 'POSTGRES_HOST'),
+    (get_port, 'POSTGRES_PORT'),
+    (get_ssl, 'POSTGRES_USE_SSL'),
+    (get_db_name, 'POSTGRES_DB_NAME'),
+    (get_db_user, 'POSTGRES_DB_USER'),
+    (get_db_pass, 'POSTGRES_DB_PASS')
+])
+def test_param_default(getter, env_var_name):
+    assert_conn_param(getter, env_var_name, '')
+
+
+@pytest.mark.parametrize('getter, env_var_name, env_value, expected_param', [
+    (get_host, 'POSTGRES_HOST', ' my.host ', 'my.host'),
+    (get_port, 'POSTGRES_PORT', ' 5432 ', 5432),
+    (get_ssl, 'POSTGRES_USE_SSL', ' no ', False),
+    (get_ssl, 'POSTGRES_USE_SSL', ' yes ', True),
+    (get_db_name, 'POSTGRES_DB_NAME', 'quantumleap\n', 'quantumleap'),
+    (get_db_user, 'POSTGRES_DB_USER', '\tquantumleap', 'quantumleap'),
+    (get_db_pass, 'POSTGRES_DB_PASS', ' p4ss ', 'p4ss')
+])
+def test_param(getter, env_var_name, env_value, expected_param):
+    assert_conn_param(getter, env_var_name, env_value, expected_param)

src/translators/tests/test_timescale_insert.py

@@ -1,5 +1,4 @@
 from datetime import datetime, timezone
-import os
 import pg8000
 import pytest
 import random
@@ -124,9 +123,6 @@ def insert(entities, fw_svc=None, fw_path=None):
 
 @pytest.fixture(scope='module')
 def with_pg8000():
-    pg_port_var = 'POSTGRES_PORT'
-    os.environ[pg_port_var] = '54320'
-
     pg8000.paramstyle = "qmark"
     t = PostgresConnectionData()
     t.read_env()
@@ -139,7 +135,6 @@ def with_pg8000():
 
     yield (pg_conn, pg_cursor)
 
-    os.environ[pg_port_var] = ''
     pg_cursor.close()
     pg_conn.close()
 

src/translators/tests/test_translator_factory.py

@@ -1,52 +1,32 @@
-import os
-import pytest
-import tempfile
-
 from translators.crate import CrateTranslator
 from translators.timescale import PostgresTranslator
-from translators.factory import QL_CONFIG_ENV_VAR, translator_for
-
-
-def write_config() -> str:
-    temp = tempfile.TemporaryFile(mode='w+t')
-    try:
-        temp.writelines('')
-        return temp.name
-    finally:
-        temp.close()
+from translators.factory import translator_for
 
 
-@pytest.fixture(scope='module')
-def with_config():
-    path = os.path.join(os.path.dirname(__file__), 'ql-config.yml')
-    os.environ[QL_CONFIG_ENV_VAR] = path
-    pg_port_var = 'POSTGRES_PORT'
-    os.environ[pg_port_var] = '54320'
-    yield {}
-    os.environ[QL_CONFIG_ENV_VAR] = ''
-    os.environ[pg_port_var] = ''
+# NOTE. Config file location set by run_tests.sh:
+#   QL_CONFIG='src/translators/tests/ql-config.yml'
 
 
-def test_tenant1(with_config):
+def test_tenant1():
     with translator_for('t1') as t:
         assert isinstance(t, PostgresTranslator)
 
 
-def test_tenant2(with_config):
+def test_tenant2():
     with translator_for('t2') as t:
         assert isinstance(t, CrateTranslator)
 
 
-def test_tenant3(with_config):
+def test_tenant3():
     with translator_for('t3') as t:
         assert isinstance(t, PostgresTranslator)
 
 
-def test_unknown_tenant(with_config):
+def test_unknown_tenant():
     with translator_for('not-in-config') as t:
         assert isinstance(t, CrateTranslator)
 
 
-def test_no_tenant(with_config):
+def test_no_tenant():
     with translator_for(None) as t:
         assert isinstance(t, CrateTranslator)

src/translators/timescale.py

@@ -1,13 +1,12 @@
 from contextlib import contextmanager
 from datetime import datetime
-import logging
 import pg8000
-import os
 
 import geocoding.geojson.wktcodec
 from geocoding.slf.geotypes import *
 import geocoding.slf.wktcodec
 from translators import base_translator
+from utils.cfgreader import *
 from utils.common import iter_entity_attrs
 
 
@@ -79,30 +78,15 @@ def __init__(self, host='timescale', port=5432, use_ssl=False,
         self.db_user = db_user
         self.db_pass = db_pass
 
-    def read_env(self):
-        env_var = os.environ.get('POSTGRES_HOST')
-        if env_var:
-            self.host = env_var
-
-        env_var = os.environ.get('POSTGRES_PORT')
-        if env_var:
-            self.port = int(env_var)
-
-        env_var = os.environ.get('POSTGRES_USE_SSL')
-        if env_var:
-            self.use_ssl = env_var.strip().lower() in ('true', 'yes', '1', 't')
-
-        env_var = os.environ.get('POSTGRES_DB_NAME')
-        if env_var:
-            self.db_name = env_var
-
-        env_var = os.environ.get('POSTGRES_DB_USER')
-        if env_var:
-            self.db_user = env_var
-
-        env_var = os.environ.get('POSTGRES_DB_PASS')
-        if env_var:
-            self.db_pass = env_var
+    def read_env(self, env: dict = os.environ):
+        r = EnvReader(env, log=logging.getLogger(__name__).info)
+        self.host = r.read(StrVar('POSTGRES_HOST', self.host))
+        self.port = r.read(IntVar('POSTGRES_PORT', self.port))
+        self.use_ssl = r.read(BoolVar('POSTGRES_USE_SSL', self.use_ssl))
+        self.db_name = r.read(StrVar('POSTGRES_DB_NAME', self.db_name))
+        self.db_user = r.read(StrVar('POSTGRES_DB_USER', self.db_user))
+        self.db_pass = r.read(StrVar('POSTGRES_DB_PASS', self.db_pass,
+                                     mask_value=True))
 
 
 class PostgresTranslator(base_translator.BaseTranslator):