The intention was to have the charm set environment variables (maintaining some agnosticism about turnip itself), but provide defaults in config.yaml.
On Wed, Feb 18, 2015 at 11:53 AM, Colin Watson <[email protected]> wrote: > Review: Approve > > Looks generally sensible, although there are a few things I'd suggest > tweaking before landing this. Are you intending to have the charm write > out a config.yaml or to have it set environment variables? > > Diff comments: > > > === modified file '.bzrignore' > > --- .bzrignore 2015-01-19 08:58:30 +0000 > > +++ .bzrignore 2015-02-17 21:40:52 +0000 > > @@ -10,3 +10,4 @@ > > build > > *.egg > > dist > > +*.log > > \ No newline at end of file > > > > === added file 'apiserver.tac' > > --- apiserver.tac 1970-01-01 00:00:00 +0000 > > +++ apiserver.tac 2015-02-17 21:40:52 +0000 > > @@ -0,0 +1,25 @@ > > +# You can run this .tac file directly with: > > +# twistd -ny apiserver.tac > > +from __future__ import ( > > + absolute_import, > > + print_function, > > + unicode_literals, > > + ) > > + > > +from twisted.web import server > > +from twisted.application import service, internet > > The Launchpad style guide would have this as: > > from twisted.application import ( > service, > internet, > ) > > We should probably follow the same style in turnip throughout. > > > + > > +from turnip.api import TurnipAPIResource > > +from turnip.config import TurnipConfig > > + > > + > > +def getAPIService(): > > + """Return an API service.""" > > + > > + config = TurnipConfig() > > + api_site = server.Site(TurnipAPIResource(config.get('REPO_STORE'))) > > + return internet.TCPServer(config.get('REPO_API_PORT'), api_site) > > + > > +application = service.Application("Turnip API Service") > > +service = getAPIService() > > +service.setServiceParent(application) > > > > === added file 'config.yaml' > > --- config.yaml 1970-01-01 00:00:00 +0000 > > +++ config.yaml 2015-02-17 21:40:52 +0000 > > @@ -0,0 +1,9 @@ > > +AUTHENTICATION_ENDPOINT: > http://xmlrpc-private.launchpad.dev:8087/authserver > > +PACK_BACKEND_PORT: 19418 > > +PACK_FRONTEND_PORT: 9418 > > +PACK_VIRT_PORT: 19419 > > +REPO_API_PORT: 19417 > > +SMART_HTTP_PORT: 9419 > > +SMART_SSH_PORT: 9422 > > +REPO_STORE: /var/tmp/git.launchpad.dev > > +VIRTINFO_ENDPOINT: http://localhost:6543/githosting > > \ No newline at end of file > > Maybe downcase all these; YAML style usually tends towards lower-case, I > think, and it matches juju style better too. This would probably entail > having TurnipConfig upper-case key names before checking for them in the > environment. > > > > > === added file 'httpserver.tac' > > --- httpserver.tac 1970-01-01 00:00:00 +0000 > > +++ httpserver.tac 2015-02-17 21:40:52 +0000 > > @@ -0,0 +1,24 @@ > > +# You can run this .tac file directly with: > > +# twistd -ny httpserver.tac > > +from __future__ import unicode_literals > > + > > +from twisted.application import service, internet > > +from twisted.web import server > > + > > +from turnip.config import TurnipConfig > > +from turnip.pack.http import SmartHTTPFrontendResource > > + > > + > > +def getSmartHTTPService(): > > + """Return a SmartHTTP frontend service.""" > > + > > + config = TurnipConfig() > > + smarthttp_site = server.Site( > > + SmartHTTPFrontendResource(b'localhost', > > + config.get('PACK_VIRT_PORT'), > > + config.get('VIRTINFO_ENDPOINT'))) > > + return internet.TCPServer(config.get('SMART_HTTP_PORT'), > smarthttp_site) > > + > > +application = service.Application("Turnip SmartHTTP Service") > > +service = getSmartHTTPService() > > +service.setServiceParent(application) > > > > === added file 'packbackendserver.tac' > > --- packbackendserver.tac 1970-01-01 00:00:00 +0000 > > +++ packbackendserver.tac 2015-02-17 21:40:52 +0000 > > @@ -0,0 +1,25 @@ > > +# You can run this .tac file directly with: > > +# twistd -ny packbackendserver.tac > > + > > +from __future__ import ( > > + absolute_import, > > + print_function, > > + unicode_literals, > > + ) > > + > > +from twisted.application import service, internet > > + > > +from turnip.config import TurnipConfig > > +from turnip.pack.git import PackBackendFactory > > + > > + > > +def getPackBackendService(): > > + """Return a PackBackendFactory service.""" > > + > > + config = TurnipConfig() > > + return internet.TCPServer(config.get('PACK_BACKEND_PORT'), > > + > PackBackendFactory(config.get('REPO_STORE'))) > > + > > +application = service.Application("Turnip Pack Backend Service") > > +service = getPackBackendService() > > +service.setServiceParent(application) > > > > === added file 'packfrontendserver.tac' > > --- packfrontendserver.tac 1970-01-01 00:00:00 +0000 > > +++ packfrontendserver.tac 2015-02-17 21:40:52 +0000 > > @@ -0,0 +1,27 @@ > > +# You can run this .tac file directly with: > > +# twistd -ny packfrontendserver.tac > > + > > +from __future__ import ( > > + absolute_import, > > + print_function, > > + unicode_literals, > > + ) > > + > > +from twisted.application import service, internet > > + > > +from turnip.config import TurnipConfig > > +from turnip.pack.git import PackFrontendFactory > > + > > + > > +def getPackFrontendService(): > > + """Return a PackFrontend Service.""" > > + > > + config = TurnipConfig() > > + return internet.TCPServer( > > + config.get('PACK_FRONTEND_PORT'), > > + PackFrontendFactory('localhost', > > + config.get('PACK_VIRT_PORT'))) > > + > > +application = service.Application("Turnip Pack Frontend Service") > > +service = getPackFrontendService() > > +service.setServiceParent(application) > > > > === modified file 'setup.py' > > --- setup.py 2015-01-19 09:51:28 +0000 > > +++ setup.py 2015-02-17 21:40:52 +0000 > > @@ -26,6 +26,7 @@ > > download_url='https://launchpad.net/turnip/+download', > > install_requires=[ > > 'lazr.sshserver', > > + 'PyYAML', > > 'Twisted', > > 'zope.interface', > > ], > > > > === added file 'sshserver.tac' > > --- sshserver.tac 1970-01-01 00:00:00 +0000 > > +++ sshserver.tac 2015-02-17 21:40:52 +0000 > > @@ -0,0 +1,37 @@ > > +# You can run this .tac file directly with: > > +# twistd -ny sshserver.tac > > + > > +from __future__ import ( > > + absolute_import, > > + print_function, > > + unicode_literals, > > + ) > > + > > +import os > > + > > +from twisted.application import service > > + > > +from turnip.config import TurnipConfig > > +from turnip.pack.ssh import SmartSSHService > > + > > + > > +def getSmartSSHService(): > > + > > + config = TurnipConfig() > > + data_dir = os.path.join( > > + os.path.dirname(__file__), "turnip", "pack", "tests", "data") > > + log_path = config.get('TURNIP_LOG_DIR') > > TURNIP_LOG_DIR doesn't seem to be in config.yaml. I know TurnipConfig > falls back to the empty string, but it should probably be in config.yaml > anyway for explicitness. > > > + > > + return SmartSSHService( > > + b'localhost', config.get('PACK_VIRT_PORT'), > > + config.get('AUTHENTICATION_ENDPOINT'), > > + private_key_path=os.path.join(data_dir, "ssh-host-key"), > > + public_key_path=os.path.join(data_dir, "ssh-host-key.pub"), > > + main_log='turnip', access_log=os.path.join(log_path, > 'turnip.access'), > > + access_log_path=os.path.join(log_path, 'turnip-access.log'), > > + strport=b'tcp:{}'.format(config.get('SMART_SSH_PORT'))) > > + > > + > > +application = service.Application("Turnip SmartSSH Service") > > +service = getSmartSSHService() > > +service.setServiceParent(application) > > > > === added file 'turnip/config.py' > > --- turnip/config.py 1970-01-01 00:00:00 +0000 > > +++ turnip/config.py 2015-02-17 21:40:52 +0000 > > @@ -0,0 +1,18 @@ > > +from __future__ import unicode_literals > > + > > +import os > > + > > +import yaml > > + > > + > > +class TurnipConfig(object): > > + """Return configuration from environment or defaults.""" > > + > > + def __init__(self): > > + """Load default configuration from config.yaml""" > > + config_file = open(os.path.join( > > + os.path.dirname(os.path.dirname(__file__)), 'config.yaml')) > > + self.defaults = yaml.load(config_file) > > + > > + def get(self, key): > > + return os.getenv(key) or self.defaults.get(key) or '' > > > > === modified file 'turnipserver.py' > > --- turnipserver.py 2015-02-04 10:28:19 +0000 > > +++ turnipserver.py 2015-02-17 21:40:52 +0000 > > @@ -10,6 +10,7 @@ > > from twisted.web import server > > > > from turnip.api import TurnipAPIResource > > +from turnip.config import TurnipConfig > > from turnip.pack.git import ( > > PackBackendFactory, > > PackFrontendFactory, > > @@ -18,34 +19,44 @@ > > from turnip.pack.http import SmartHTTPFrontendResource > > from turnip.pack.ssh import SmartSSHService > > > > -AUTHENTICATION_ENDPOINT = ( > > - b'http://xmlrpc-private.launchpad.dev:8087/authserver') > > -LOG_PATH = os.getenv('TURNIP_LOG_DIR', '') > > -REPO_STORE = '/var/tmp/git.launchpad.dev' > > -VIRTINFO_ENDPOINT = b'http://localhost:6543/githosting' > > data_dir = os.path.join( > > os.path.dirname(__file__), "turnip", "pack", "tests", "data") > > - > > +config = TurnipConfig() > > + > > +LOG_PATH = config.get('TURNIP_LOG_DIR') > > +PACK_VIRT_PORT = config.get('PACK_VIRT_PORT') > > +PACK_BACKEND_PORT = config.get('PACK_BACKEND_PORT') > > +REPO_STORE = config.get('REPO_STORE') > > +VIRTINFO_ENDPOINT = config.get('VIRTINFO_ENDPOINT') > > + > > +# turnipserver.py is preserved for convenience in development, services > > +# in production are run in separate processes. > > +# > > # Start a pack storage service on 19418, pointed at by a pack frontend > > # on 9418 (the default git:// port), a smart HTTP frontend on 9419, and > > # a smart SSH frontend on 9422. An API service runs on 19417. > > -reactor.listenTCP(19418, PackBackendFactory(REPO_STORE)) > > -reactor.listenTCP( > > - 19419, PackVirtFactory('localhost', 19418, VIRTINFO_ENDPOINT)) > > -reactor.listenTCP(9418, PackFrontendFactory('localhost', 19419)) > > +reactor.listenTCP(PACK_BACKEND_PORT, > > + PackBackendFactory(REPO_STORE)) > > +reactor.listenTCP(PACK_VIRT_PORT, > > + PackVirtFactory('localhost', > > + PACK_BACKEND_PORT, > > + VIRTINFO_ENDPOINT)) > > +reactor.listenTCP(config.get('PACK_FRONTEND_PORT'), > > + PackFrontendFactory('localhost', > > + PACK_VIRT_PORT)) > > smarthttp_site = server.Site( > > - SmartHTTPFrontendResource(b'localhost', 19419, VIRTINFO_ENDPOINT)) > > -reactor.listenTCP(9419, smarthttp_site) > > + SmartHTTPFrontendResource(b'localhost', PACK_VIRT_PORT, > VIRTINFO_ENDPOINT)) > > +reactor.listenTCP(config.get('SMART_HTTP_PORT'), smarthttp_site) > > smartssh_service = SmartSSHService( > > - b'localhost', 19419, AUTHENTICATION_ENDPOINT, > > + b'localhost', 19419, config.get('AUTHENTICATION_ENDPOINT'), > > 19419 should be unhardcoded too. > > > private_key_path=os.path.join(data_dir, "ssh-host-key"), > > public_key_path=os.path.join(data_dir, "ssh-host-key.pub"), > > main_log='turnip', access_log=os.path.join(LOG_PATH, > 'turnip.access'), > > access_log_path=os.path.join(LOG_PATH, 'turnip-access.log'), > > - strport=b'tcp:9422') > > + strport=b'tcp:{}'.format(config.get('SMART_SSH_PORT'))) > > smartssh_service.startService() > > > > api_site = server.Site(TurnipAPIResource(REPO_STORE)) > > -reactor.listenTCP(19417, api_site) > > +reactor.listenTCP(config.get('REPO_API_PORT'), api_site) > > > > reactor.run() > > > > === modified file 'versions.cfg' > > --- versions.cfg 2015-01-22 03:16:48 +0000 > > +++ versions.cfg 2015-02-17 21:40:52 +0000 > > @@ -12,6 +12,7 @@ > > pyasn1 = 0.1.6 > > pycrypto = 2.6 > > python-mimeparse = 0.1.4 > > +PyYAML = 3.11 > > setuptools = 0.6c11 > > testtools = 0.9.30 > > Twisted = 13.0.0 > > You probably need to change requirements.txt similarly, for pip. > > > > > === added file 'virtserver.tac' > > --- virtserver.tac 1970-01-01 00:00:00 +0000 > > +++ virtserver.tac 2015-02-17 21:40:52 +0000 > > @@ -0,0 +1,25 @@ > > +from __future__ import ( > > + absolute_import, > > + print_function, > > + unicode_literals, > > + ) > > + > > +from twisted.application import service, internet > > + > > +from turnip.config import TurnipConfig > > +from turnip.pack.git import PackVirtFactory > > + > > + > > +def getPackVirtService(): > > + """Return a PackVirt Service.""" > > + > > + config = TurnipConfig() > > + return internet.TCPServer( > > + config.get('PACK_VIRT_PORT'), > > + PackVirtFactory('localhost', > > + config.get('PACK_BACKEND_PORT'), > > + config.get('VIRTINFO_ENDPOINT'))) > > + > > +application = service.Application("Turnip Pack Virt Service") > > +service = getPackVirtService() > > +service.setServiceParent(application) > > > > > -- > https://code.launchpad.net/~blr/turnip/turnip-config/+merge/250076 > You are the owner of lp:~blr/turnip/turnip-config. > -- Kit Randel Canonical - Ubuntu Engineering - Launchpad & Continuous Integration Team https://code.launchpad.net/~blr/turnip/turnip-config/+merge/250076 Your team Launchpad code reviewers is subscribed to branch lp:turnip. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
