Merge pull request tableau#347 from tableau/dev

Merge 'dev' to 'master'

CHANGELOG

@@ -1,5 +1,11 @@
 # Changelog
 
+## v0.8.7
+
+### Improvements
+
+- Enabled the use of environment variables in the config file.
+
 ## v0.8.6
 
 ### Fixes

docs/server-config.md

@@ -50,6 +50,9 @@ Configuration file consists of settings for TabPy itself and Python logger
 settings. You should only set parameters if you need different values than
 the defaults.
 
+Environment variables can be used in the config file. Any instances of
+`%(ENV_VAR)s` will be replaced by the value of the environment variable `ENV_VAR`.
+
 TabPy parameters explained below, the logger documentation can be found
 at [`logging.config` documentation page](https://docs.python.org/3.6/library/logging.config.html).
 

tabpy/VERSION

@@ -1 +1 @@
-0.8.6
+0.8.7

tabpy/tabpy_server/app/app.py

@@ -160,7 +160,7 @@ def _parse_config(self, config_file):
         self.python_service = None
         self.credentials = {}
 
-        parser = configparser.ConfigParser()
+        parser = configparser.ConfigParser(os.environ)
 
         if os.path.isfile(config_file):
             with open(config_file) as f:
@@ -172,8 +172,7 @@ def _parse_config(self, config_file):
 
         def set_parameter(settings_key,
                           config_key,
-                          default_val=None,
-                          check_env_var=False):
+                          default_val=None):
             key_is_set = False
 
             if config_key is not None and\
@@ -184,17 +183,7 @@ def set_parameter(settings_key,
                 logger.debug(
                     f'Parameter {settings_key} set to '
                     f'"{self.settings[settings_key]}" '
-                    'from config file')
-
-            if not key_is_set and check_env_var:
-                val = os.getenv(config_key)
-                if val is not None:
-                    self.settings[settings_key] = val
-                    key_is_set = True
-                    logger.debug(
-                        f'Parameter {settings_key} set to '
-                        f'"{self.settings[settings_key]}" '
-                        'from environment variable')
+                    'from config file or environment variable')
 
             if not key_is_set and default_val is not None:
                 self.settings[settings_key] = default_val
@@ -209,13 +198,14 @@ def set_parameter(settings_key,
                     f'Parameter {settings_key} is not set')
 
         set_parameter(SettingsParameters.Port, ConfigParameters.TABPY_PORT,
-                      default_val=9004, check_env_var=True)
+                      default_val=9004)
         set_parameter(SettingsParameters.ServerVersion, None,
                       default_val=__version__)
 
         set_parameter(SettingsParameters.EvaluateTimeout,
                       ConfigParameters.TABPY_EVALUATE_TIMEOUT,
                       default_val=30)
+
         try:
             self.settings[SettingsParameters.EvaluateTimeout] = float(
                 self.settings[SettingsParameters.EvaluateTimeout])
@@ -229,8 +219,7 @@ def set_parameter(settings_key,
         set_parameter(SettingsParameters.UploadDir,
                       ConfigParameters.TABPY_QUERY_OBJECT_PATH,
                       default_val=os.path.join(pkg_path,
-                                               'tmp', 'query_objects'),
-                      check_env_var=True)
+                                               'tmp', 'query_objects'))
         if not os.path.exists(self.settings[SettingsParameters.UploadDir]):
             os.makedirs(self.settings[SettingsParameters.UploadDir])
 
@@ -251,8 +240,7 @@ def set_parameter(settings_key,
         # last dependence on batch/shell script
         set_parameter(SettingsParameters.StateFilePath,
                       ConfigParameters.TABPY_STATE_PATH,
-                      default_val=os.path.join(pkg_path, 'tabpy_server'),
-                      check_env_var=True)
+                      default_val=os.path.join(pkg_path, 'tabpy_server'))
         self.settings[SettingsParameters.StateFilePath] = os.path.realpath(
             os.path.normpath(
                 os.path.expanduser(

tests/unit/server_tests/test_config.py

@@ -6,15 +6,7 @@
 from tabpy.tabpy_server.app.util import validate_cert
 from tabpy.tabpy_server.app.app import TabPyApp
 
-from unittest.mock import patch, call
-
-
-def assert_raises_runtime_error(message, fn, args={}):
-    try:
-        fn(*args)
-        assert False
-    except RuntimeError as err:
-        assert err.args[0] == message
+from unittest.mock import patch
 
 
 class TestConfigEnvironmentCalls(unittest.TestCase):
@@ -32,16 +24,12 @@ def test_no_config_file(self, mock_os,
         pkg_path = os.path.dirname(tabpy.__file__)
         obj_path = os.path.join(pkg_path, 'tmp', 'query_objects')
         state_path = os.path.join(pkg_path, 'tabpy_server')
-
-        mock_os.getenv.side_effect = [9004, obj_path, state_path]
+        mock_os.environ = {
+            'TABPY_PORT': '9004', 'TABPY_QUERY_OBJECT_PATH': obj_path,
+            'TABPY_STATE_PATH': state_path}
 
         TabPyApp(None)
 
-        getenv_calls = [
-            call('TABPY_PORT'),
-            call('TABPY_QUERY_OBJECT_PATH'),
-            call('TABPY_STATE_PATH')]
-        mock_os.getenv.assert_has_calls(getenv_calls, any_order=True)
         self.assertEqual(len(mock_psws.mock_calls), 1)
         self.assertEqual(len(mock_tabpy_state.mock_calls), 1)
         self.assertEqual(len(mock_path_exists.mock_calls), 1)
@@ -89,15 +77,12 @@ def test_config_file_present(self, mock_os, mock_path_exists,
         config_file.close()
 
         mock_parse_arguments.return_value = Namespace(config=config_file.name)
-
-        mock_os.getenv.side_effect = [1234]
         mock_os.path.realpath.return_value = 'bar'
+        mock_os.environ = {'TABPY_PORT': '1234'}
 
         app = TabPyApp(config_file.name)
-        getenv_calls = [call('TABPY_PORT')]
 
-        mock_os.getenv.assert_has_calls(getenv_calls, any_order=True)
-        self.assertEqual(app.settings['port'], 1234)
+        self.assertEqual(app.settings['port'], '1234')
         self.assertEqual(app.settings['server_version'],
                          open('tabpy/VERSION').read().strip())
         self.assertEqual(app.settings['upload_dir'], 'foo')
@@ -138,8 +123,28 @@ def test_custom_evaluate_timeout_invalid(self, mock_state,
         app = TabPyApp(self.config_file.name)
         self.assertEqual(app.settings['evaluate_timeout'], 30.0)
 
+    @patch('tabpy.tabpy_server.app.app.os')
+    @patch('tabpy.tabpy_server.app.app.os.path.exists', return_value=True)
+    @patch('tabpy.tabpy_server.app.app._get_state_from_file')
+    @patch('tabpy.tabpy_server.app.app.TabPyState')
+    def test_env_variables_in_config(self, mock_state, mock_get_state,
+                                     mock_path_exists, mock_os):
+        mock_os.environ = {'foo': 'baz'}
+        config_file = self.config_file
+        config_file.write('[TabPy]\n'
+                          'TABPY_PORT = %(foo)sbar'.encode())
+        config_file.close()
+
+        app = TabPyApp(self.config_file.name)
+        self.assertEqual(app.settings['port'], 'bazbar')
+
 
 class TestTransferProtocolValidation(unittest.TestCase):
+    def assertTabPyAppRaisesRuntimeError(self, expected_message):
+        with self.assertRaises(RuntimeError) as err:
+            TabPyApp(self.fp.name)
+        self.assertEqual(err.exception.args[0], expected_message)
+
     @staticmethod
     def mock_isfile(target_file, existing_files):
         if target_file in existing_files:
@@ -174,10 +179,9 @@ def test_https_without_cert_and_key(self):
                       "TABPY_TRANSFER_PROTOCOL = https")
         self.fp.close()
 
-        assert_raises_runtime_error(
-            'Error using HTTPS: The parameter(s) TABPY_CERTIFICATE_FILE '
-            'and TABPY_KEY_FILE must be set.',
-            TabPyApp, {self.fp.name})
+        self.assertTabPyAppRaisesRuntimeError('Error using HTTPS: The paramete'
+                                              'r(s) TABPY_CERTIFICATE_FILE and'
+                                              ' TABPY_KEY_FILE must be set.')
 
     def test_https_without_cert(self):
         self.fp.write(
@@ -186,19 +190,18 @@ def test_https_without_cert(self):
             "TABPY_KEY_FILE = foo")
         self.fp.close()
 
-        assert_raises_runtime_error('Error using HTTPS: The parameter(s) '
-                                    'TABPY_CERTIFICATE_FILE must be set.',
-                                    TabPyApp, {self.fp.name})
+        self.assertTabPyAppRaisesRuntimeError(
+            'Error using HTTPS: The parameter(s) TABPY_CERTIFICATE_FILE must '
+            'be set.')
 
     def test_https_without_key(self):
         self.fp.write("[TabPy]\n"
                       "TABPY_TRANSFER_PROTOCOL = https\n"
                       "TABPY_CERTIFICATE_FILE = foo")
         self.fp.close()
 
-        assert_raises_runtime_error('Error using HTTPS: The parameter(s) '
-                                    'TABPY_KEY_FILE must be set.',
-                                    TabPyApp, {self.fp.name})
+        self.assertTabPyAppRaisesRuntimeError(
+            'Error using HTTPS: The parameter(s) TABPY_KEY_FILE must be set.')
 
     @patch('tabpy.tabpy_server.app.app.os.path')
     def test_https_cert_and_key_file_not_found(self, mock_path):
@@ -211,10 +214,9 @@ def test_https_cert_and_key_file_not_found(self, mock_path):
         mock_path.isfile.side_effect = lambda x: self.mock_isfile(
             x, {self.fp.name})
 
-        assert_raises_runtime_error(
+        self.assertTabPyAppRaisesRuntimeError(
             'Error using HTTPS: The parameter(s) TABPY_CERTIFICATE_FILE and '
-            'TABPY_KEY_FILE must point to an existing file.',
-            TabPyApp, {self.fp.name})
+            'TABPY_KEY_FILE must point to an existing file.')
 
     @patch('tabpy.tabpy_server.app.app.os.path')
     def test_https_cert_file_not_found(self, mock_path):
@@ -227,10 +229,9 @@ def test_https_cert_file_not_found(self, mock_path):
         mock_path.isfile.side_effect = lambda x: self.mock_isfile(
             x, {self.fp.name, 'bar'})
 
-        assert_raises_runtime_error(
+        self.assertTabPyAppRaisesRuntimeError(
             'Error using HTTPS: The parameter(s) TABPY_CERTIFICATE_FILE '
-            'must point to an existing file.',
-            TabPyApp, {self.fp.name})
+            'must point to an existing file.')
 
     @patch('tabpy.tabpy_server.app.app.os.path')
     def test_https_key_file_not_found(self, mock_path):
@@ -243,10 +244,9 @@ def test_https_key_file_not_found(self, mock_path):
         mock_path.isfile.side_effect = lambda x: self.mock_isfile(
             x, {self.fp.name, 'foo'})
 
-        assert_raises_runtime_error(
+        self.assertTabPyAppRaisesRuntimeError(
             'Error using HTTPS: The parameter(s) TABPY_KEY_FILE '
-            'must point to an existing file.',
-            TabPyApp, {self.fp.name})
+            'must point to an existing file.')
 
     @patch('tabpy.tabpy_server.app.app.os.path.isfile', return_value=True)
     @patch('tabpy.tabpy_server.app.util.validate_cert')
@@ -265,6 +265,10 @@ def test_https_success(self, mock_isfile, mock_validate_cert):
 
 
 class TestCertificateValidation(unittest.TestCase):
+    def assertValidateCertRaisesRuntimeError(self, expected_message, path):
+        with self.assertRaises(RuntimeError) as err:
+            validate_cert(path)
+        self.assertEqual(err.exception.args[0], expected_message)
 
     def __init__(self, *args, **kwargs):
         super(TestCertificateValidation, self).__init__(*args, **kwargs)
@@ -275,13 +279,13 @@ def test_expired_cert(self):
         path = os.path.join(self.resources_path, 'expired.crt')
         message = ('Error using HTTPS: The certificate provided expired '
                    'on 2018-08-18 19:47:18.')
-        assert_raises_runtime_error(message, validate_cert, {path})
+        self.assertValidateCertRaisesRuntimeError(message, path)
 
     def test_future_cert(self):
         path = os.path.join(self.resources_path, 'future.crt')
         message = ('Error using HTTPS: The certificate provided is not valid '
                    'until 3001-01-01 00:00:00.')
-        assert_raises_runtime_error(message, validate_cert, {path})
+        self.assertValidateCertRaisesRuntimeError(message, path)
 
     def test_valid_cert(self):
         path = os.path.join(self.resources_path, 'valid.crt')