launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/maas/mipf-optional-proxy into lp:maas.

Commit message:
Make the celery task 'import-pxe-files' take an optional http proxy.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~julian-edwards/maas/mipf-optional-proxy/+merge/130964
-- 
https://code.launchpad.net/~julian-edwards/maas/mipf-optional-proxy/+merge/130964
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/maas/mipf-optional-proxy into lp:maas.

=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py	2012-10-23 08:27:11 +0000
+++ src/provisioningserver/tasks.py	2012-10-23 09:36:07 +0000
@@ -23,6 +23,7 @@
     'write_full_dns_config',
     ]
 
+import os
 from subprocess import (
     CalledProcessError,
     check_call,
@@ -375,5 +376,9 @@
 # =====================================================================
 
 @task
-def import_pxe_files():
-    check_call(['maas-import-pxe-files'])
+def import_pxe_files(http_proxy=None):
+    env = dict(os.environ)
+    if http_proxy is not None:
+        env['http_proxy'] = http_proxy
+        env['https_proxy'] = http_proxy
+    check_call(['maas-import-pxe-files'], env=env)

=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py	2012-10-23 08:27:11 +0000
+++ src/provisioningserver/tests/test_tasks.py	2012-10-23 09:36:07 +0000
@@ -24,8 +24,8 @@
 from apiclient.creds import convert_tuple_to_string
 from apiclient.maas_client import MAASClient
 from apiclient.testing.credentials import make_api_credentials
+from celery.app import app_or_default
 from celery.task import Task
-from celery.app import app_or_default
 from maastesting.celery import CeleryFixture
 from maastesting.factory import factory
 from maastesting.fakemethod import (
@@ -33,7 +33,10 @@
     MultiFakeMethod,
     )
 from maastesting.matchers import ContainsAll
-from mock import Mock
+from mock import (
+    ANY,
+    Mock,
+    )
 from netaddr import IPNetwork
 from provisioningserver import (
     auth,
@@ -543,5 +546,19 @@
     def test_import_pxe_files(self):
         recorder = self.patch(tasks, 'check_call', Mock())
         import_pxe_files()
-        recorder.assert_called_once_with(['maas-import-pxe-files'])
+        recorder.assert_called_once_with(['maas-import-pxe-files'], env=ANY)
         self.assertIsInstance(import_pxe_files, Task)
+
+    def test_import_pxe_files_preserves_environment(self):
+        recorder = self.patch(tasks, 'check_call', Mock())
+        import_pxe_files()
+        recorder.assert_called_once_with(
+            ['maas-import-pxe-files'], env=os.environ)
+
+    def test_import_pxe_files_sets_proxy(self):
+        recorder = self.patch(tasks, 'check_call', Mock())
+        proxy = factory.getRandomString()
+        import_pxe_files(http_proxy=proxy)
+        expected_env = dict(os.environ, http_proxy=proxy, https_proxy=proxy)
+        recorder.assert_called_once_with(
+            ['maas-import-pxe-files'], env=expected_env)