Code review comment for lp:~rockstar/launchpad/fix-windmill-canonicalurl

Revision history for this message
Björn Tillenius (bjornt) wrote :

On Thu, Jan 14, 2010 at 12:10:25AM -0000, Paul Hummer wrote:
> === modified file 'lib/canonical/testing/layers.py'
> --- lib/canonical/testing/layers.py 2009-12-16 09:53:34 +0000
> +++ lib/canonical/testing/layers.py 2010-01-14 00:10:24 +0000
> @@ -89,6 +89,7 @@
> from zope.server.logger.pythonlogger import PythonLogger
> from zope.testing.testrunner.runner import FakeInputContinueGenerator
>
> +from canonical.launchpad.webapp.vhosts import allvhosts
> from canonical.lazr import pidfile
> from canonical.config import CanonicalConfig, config, dbconfig
> from canonical.database.revision import (
> @@ -1832,6 +1833,20 @@
> os.environ['WINDMILL_CONFIG_FILE'] = cls.config_file.name
> cls.shell_objects = start_windmill()
>
> + # Patch the config to provide the port number and not use https.
> + sites = (
> + ('vhost.mainsite', 'rooturl: http://launchpad.dev:8085/'),
> + ('vhost.answers', 'rooturl: http://answers.launchpad.dev:8085/'),
> + ('vhost.blueprints',
> + 'rooturl: http://blueprints.launchpad.dev:8085/'),
> + ('vhost.bugs', 'rooturl: http://bugs.launchpad.dev:8085/'),
> + ('vhost.code', 'rooturl: http://code.launchpad.dev:8085/'),
> + ('vhost.translations',
> + 'rooturl: http://translations.launchpad.dev:8085/'))
> + for site in sites:
> + config.push('windmillsettings', "\n[%s]\n%s\n" % site)
> + allvhosts.reload()
> +
> @classmethod
> @profiled
> def tearDown(cls):
> @@ -1840,6 +1855,7 @@
> if cls.config_file is not None:
> # Close the file so that it gets deleted.
> cls.config_file.close()
> + config.pop('windmillsettings')

As discussed in person, this looked a bit suspicious to me. I'm not sure
it will remove all the config section that were added. Let's confirm
that this actually works. Everything else looks good.

    review approve

--
Björn Tillenius | https://launchpad.net/~bjornt

review: Approve

« Back to merge proposal