Added support for expanding environment variables in configuration files.

bin/build-artifacts-in-docker.sh

@@ -31,6 +31,7 @@ docker run --name ${CONTAINER_NAME} -v ${ROOT_DIR}:${CONTAINER_DIR} --rm -i ${BA
     pip install Babel==2.5.3
     pip install cffi==1.11.5
     pip install PyNaCl==1.2.1
+    pip install idna==2.7
     pip install cryptography==2.1.1
     pip install -r requirements.txt
     export PYTHONPATH=${PYTHONPATH}:$(pwd)

packaging/bdist_prestoadmin.py

@@ -91,11 +91,9 @@ def package_dependencies(self, build_dir):
         thirdparty_dir = os.path.join(build_dir, 'third-party')
 
         requirements = self.distribution.install_requires
-        for requirement in requirements:
-            pip.main(['wheel',
-                      '--wheel-dir={0}'.format(thirdparty_dir),
-                      '--no-cache',
-                      requirement])
+        pip.main(['wheel',
+                  '--wheel-dir={0}'.format(thirdparty_dir),
+                  '--no-cache'] + requirements)
 
         pip.main(['install',
                   '-d',

prestoadmin/config.py

@@ -53,6 +53,12 @@ def get_conf_from_properties_data(data):
         if len(line) > 0 and line[0] not in COMMENT_CHARS:
             pair = split_to_pair(line)
             props[pair[0]] = pair[1]
+    return expand_environment_variables(props)
+
+
+def expand_environment_variables(props):
+    for key in props:
+        props[key] = os.path.expandvars(props[key])
     return props
 
 

tests/product/prestoadmin_installer.py

@@ -115,7 +115,8 @@ def _build_installer_in_docker(self, cluster, online_installer=None,
                 # use explicit versions of dependent packages
                 'pip install --upgrade pycparser==2.18 cffi==1.11.5\n'
                 'pip install --upgrade pycparser==2.18 PyNaCl==1.2.1\n'
-                'pip install --upgrade pycparser==2.18 cryptography==2.1.1\n'
+                'pip install idna==2.7\n'
+                'pip install cryptography==2.1.1\n'
                 'pip install --upgrade pip==7.1.2\n'
                 'pip install --upgrade wheel==0.23.0\n'
                 'pip install --upgrade setuptools==20.1.1\n'

tests/product/resources/configuration_show_default_master_slave1.txt

@@ -1,3 +1,4 @@
+
 master: Configuration file at /etc/presto/node.properties:
 node.id=.*
 catalog.config-dir=/etc/presto/catalog
@@ -32,6 +33,11 @@ query.max-memory-per-node=512MB
 query.max-memory=50GB
 
 
+master: Configuration file at /etc/presto/log.properties:
+# Enable verbose logging from Presto
+#com.facebook.presto=DEBUG
+
+
 slave1: Configuration file at /etc/presto/node.properties:
 node.id=.*
 catalog.config-dir=/etc/presto/catalog
@@ -62,3 +68,8 @@ discovery.uri=http://master:7070
 http-server.http.port=7070
 query.max-memory-per-node=512MB
 query.max-memory=50GB
+
+
+slave1: Configuration file at /etc/presto/log.properties:
+# Enable verbose logging from Presto
+#com.facebook.presto=DEBUG

tests/product/resources/configuration_show_default_slave2_slave3.txt

@@ -30,6 +30,11 @@ query.max-memory-per-node=512MB
 query.max-memory=50GB
 
 
+slave2: Configuration file at /etc/presto/log.properties:
+# Enable verbose logging from Presto
+#com.facebook.presto=DEBUG
+
+
 slave3: Configuration file at /etc/presto/node.properties:
 node.id=.*
 catalog.config-dir=/etc/presto/catalog
@@ -60,3 +65,8 @@ discovery.uri=http://master:7070
 http-server.http.port=7070
 query.max-memory-per-node=512MB
 query.max-memory=50GB
+
+
+slave3: Configuration file at /etc/presto/log.properties:
+# Enable verbose logging from Presto
+#com.facebook.presto=DEBUG

tests/unit/resources/environment_variable.properties

@@ -0,0 +1 @@
+a=${ENVIRONMENT_VARIABLE}

tests/unit/resources/valid.config

@@ -1,3 +1,4 @@
 prop1
 prop2
-prop3
+prop3
+${ENVIRONMENT_VARIABLE}

tests/unit/resources/valid.properties

@@ -9,4 +9,4 @@ f===6
 g:= 7
 h=:8
 i   = 9
-
+j=${ENVIRONMENT_VARIABLE}

tests/unit/test_config.py

@@ -58,14 +58,21 @@ def test_invalid_json(self):
     def test_get_config(self):
         config_file = os.path.join(DIR, 'resources', 'valid.config')
         conf = config.get_conf_from_config_file(config_file)
-        self.assertEqual(conf, ['prop1', 'prop2', 'prop3'])
+        self.assertEqual(conf, ['prop1', 'prop2', 'prop3', '${ENVIRONMENT_VARIABLE}'])
 
     def test_get_properties(self):
         config_file = os.path.join(DIR, 'resources', 'valid.properties')
         conf = config.get_conf_from_properties_file(config_file)
         self.assertEqual(conf, {'a': '1', 'b': '2', 'c': '3',
                                 'd\\=': '4', 'e\\:': '5', 'f': '==6',
-                                'g': '= 7', 'h': ':8', 'i': '9'})
+                                'g': '= 7', 'h': ':8', 'i': '9', 'j': '${ENVIRONMENT_VARIABLE}'})
+
+    @patch('prestoadmin.config.os.path.expandvars', return_value='environment_variable_value')
+    def test_expending_environment_variable(self, environment_variables):
+        config_file = os.path.join(DIR, 'resources', 'environment_variable.properties')
+        conf = config.get_conf_from_properties_file(config_file)
+        self.assertTrue(environment_variables.called)
+        self.assertEqual({'a': 'environment_variable_value'}, conf)
 
     @patch('__builtin__.open')
     def test_get_properties_ignores_whitespace(self, open_mock):