Address code review comments

app/client/build_runner.py

@@ -20,7 +20,7 @@ class BuildRunner(object):
     build results.
 
     Example usage pattern:
-    >>> runner = BuildRunner('http(s)://mymaster.net:123', {'type':'git', 'url':'https://github.com/box/StatusWolf.git'})
+    >>> runner = BuildRunner('https://mymaster.net:123', {'type':'git', 'url':'https://github.com/box/StatusWolf.git'})
     >>> runner.run()
     """
 

app/client/cluster_api_client.py

@@ -3,11 +3,11 @@
 from typing import Any, Callable, Dict, List, Optional
 
 from app.master.build import BuildStatus
+from app.util.conf.configuration import Configuration
 from app.util import log, poll
 from app.util.network import Network
 from app.util.secret import Secret
 from app.util.url_builder import UrlBuilder
-from app.util.conf.configuration import Configuration
 
 
 class ClusterAPIClient(object):
@@ -31,7 +31,7 @@ def _ensure_url_has_scheme(self, url):
         """
         url = url.strip()
         if not url.startswith('http'):
-            url = '{}://'.format(Configuration['protocol_scheme']) + url
+            url = '{}://{}'.format(Configuration['protocol_scheme'], url)
         return url
 
 

app/subcommands/service_subcommand.py

@@ -4,11 +4,11 @@
 import tornado.ioloop
 from tornado.httpserver import HTTPServer
 
+from app.util.conf.configuration import Configuration
 from app.subcommands.subcommand import Subcommand
 from app.util import fs
 from app.util.safe_thread import SafeThread
 from app.util.unhandled_exception_handler import UnhandledExceptionHandler
-from app.util.conf.configuration import Configuration
 
 
 class ServiceSubcommand(Subcommand):
@@ -61,8 +61,8 @@ def remove_pid_file():
         UnhandledExceptionHandler.singleton().add_teardown_callback(remove_pid_file)
 
     def _get_https_options(self):
-        https_cert_file = Configuration['https_cert_file'] if 'https_cert_file' in Configuration else None
-        https_key_file = Configuration['https_key_file'] if 'https_key_file' in Configuration else None
+        https_cert_file = Configuration['https_cert_file']
+        https_key_file = Configuration['https_key_file']
 
         if https_cert_file and https_key_file:
             return {

app/util/conf/base_config_loader.py

@@ -5,6 +5,7 @@
 import shutil
 
 from app.util import autoversioning, fs
+from app.util.conf.configuration import Configuration
 from app.util.conf.config_file import ConfigFile
 
 
@@ -116,6 +117,10 @@ def configure_postload(self, conf):
         conf.set('log_file', join(log_dir, conf.get('log_filename')))
         conf.set('eventlog_file', join(log_dir, conf.get('eventlog_filename')))
 
+        # Set protocol scheme (by default it is set to 'http')
+        if Configuration['https_cert_file'] and Configuration['https_key_file']:
+            conf.set('protocol_scheme', 'https')
+
     def load_from_config_file(self, config, config_filename):
         """
         :type config: Configuration

app/util/conf/master_config_loader.py

@@ -1,7 +1,6 @@
 from os.path import join
 
 from app.util.conf.base_config_loader import BaseConfigLoader
-from app.util.conf.configuration import Configuration
 
 
 class MasterConfigLoader(BaseConfigLoader):
@@ -39,9 +38,3 @@ def configure_postload(self, conf):
         # where to store results on the master
         conf.set('results_directory', join(base_directory, 'results', 'master'))
         conf.set('timings_directory', join(base_directory, 'timings', 'master'))  # timing data
-
-        # Set protocol scheme (by default its set to 'http')
-        https_cert_file = Configuration['https_cert_file']
-        https_key_file = Configuration['https_key_file']
-        if https_cert_file and https_key_file:
-            conf.set('protocol_scheme', 'https')

app/util/conf/slave_config_loader.py

@@ -1,7 +1,6 @@
 from os.path import join
 
 from app.util.conf.base_config_loader import BaseConfigLoader
-from app.util.conf.configuration import Configuration
 
 
 class SlaveConfigLoader(BaseConfigLoader):
@@ -41,9 +40,3 @@ def configure_postload(self, conf):
         # where to store results on the slave
         conf.set('results_directory', join(base_directory, 'results', 'slave'))
         conf.set('timings_directory', join(base_directory, 'timings', 'master'))  # timing data
-
-        # Set protocol scheme (by default its set to 'http')
-        https_cert_file = Configuration['https_cert_file']
-        https_key_file = Configuration['https_key_file']
-        if https_cert_file and https_key_file:
-            conf.set('protocol_scheme', 'https')