launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:fix-buildmaster-fetch-in-thread into launchpad:master.

Commit message:
Fix proxy token handling for buildd-manager changes

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/383734

Now that buildd-manager fetches files using threads, both the code used to fetch snap proxy tokens and its associated tests need some adjustments.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:fix-buildmaster-fetch-in-thread into launchpad:master.

diff --git a/lib/lp/buildmaster/interactor.py b/lib/lp/buildmaster/interactor.py
index e711862..64a54f3 100644
--- a/lib/lp/buildmaster/interactor.py
+++ b/lib/lp/buildmaster/interactor.py
@@ -6,6 +6,7 @@ __metaclass__ = type
 __all__ = [
     'BuilderInteractor',
     'extract_vitals_from_db',
+    'shut_down_default_threadpool',
     ]
 
 from collections import namedtuple
diff --git a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
index ad09ea4..8536072 100644
--- a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
+++ b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
@@ -41,7 +41,10 @@ from lp.buildmaster.enums import (
     BuildBaseImageType,
     BuildStatus,
     )
-from lp.buildmaster.interactor import BuilderInteractor
+from lp.buildmaster.interactor import (
+    BuilderInteractor,
+    shut_down_default_threadpool,
+    )
 from lp.buildmaster.interfaces.builder import (
     BuildDaemonError,
     CannotBuild,
@@ -111,7 +114,7 @@ class MakeOCIBuildMixin:
         builder.processor = job.build.processor
         slave = self.useFixture(SlaveTestHelpers()).getClientSlave()
         job.setBuilder(builder, slave)
-        self.addCleanup(slave.pool.closeCachedConnections)
+        self.addCleanup(shut_down_default_threadpool)
 
         # Taken from test_archivedependencies.py
         for component_name in ("main", "universe"):
@@ -207,7 +210,7 @@ class TestAsyncOCIRecipeBuildBehaviour(MakeOCIBuildMixin, TestCaseWithFactory):
                         Equals(b"Basic " + base64.b64encode(
                             b"admin-launchpad.test:admin-secret"))]),
                     b"Content-Type": MatchesListwise([
-                        Equals(b"application/json; charset=UTF-8"),
+                        Equals(b"application/json"),
                         ]),
                     }),
                 "content": AfterPreprocessing(json.loads, MatchesDict({
diff --git a/lib/lp/snappy/model/snapbuildbehaviour.py b/lib/lp/snappy/model/snapbuildbehaviour.py
index cbcba1d..c34534e 100644
--- a/lib/lp/snappy/model/snapbuildbehaviour.py
+++ b/lib/lp/snappy/model/snapbuildbehaviour.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """An `IBuildFarmJobBehaviour` for `SnapBuild`.
@@ -17,12 +17,15 @@ __all__ = [
 import base64
 import time
 
+from requests import Session
 from six.moves.urllib.parse import (
     urlsplit,
     urlunsplit,
     )
-import treq
-from twisted.internet import defer
+from twisted.internet import (
+    defer,
+    threads,
+    )
 from zope.component import adapter
 from zope.interface import implementer
 from zope.security.proxy import removeSecurityProxy
@@ -39,7 +42,6 @@ from lp.registry.interfaces.series import SeriesStatus
 from lp.services.config import config
 from lp.services.features import getFeatureFlag
 from lp.services.twistedsupport import cancel_on_timeout
-from lp.services.twistedsupport.treq import check_status
 from lp.snappy.interfaces.snap import (
     SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG,
     SnapBuildArchiveOwnerMismatch,
@@ -101,13 +103,14 @@ class SnapProxyMixin:
         auth_string = '{}:{}'.format(admin_username, secret).strip()
         auth_header = b'Basic ' + base64.b64encode(auth_string)
 
-        response = yield treq.post(
+        session = Session()
+        session.trust_env = False
+        response = yield threads.deferToThreadPool(
+            self._slave.reactor, self._slave.threadpool, session.post,
             url, headers={'Authorization': auth_header},
-            json={'username': proxy_username},
-            reactor=self._slave.reactor,
-            pool=self._slave.pool)
-        response = yield check_status(response)
-        token = yield treq.json_content(response)
+            json={'username': proxy_username})
+        response.raise_for_status()
+        token = response.json()
         defer.returnValue(token)
 
 
diff --git a/lib/lp/snappy/tests/test_snapbuildbehaviour.py b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
index f851008..dfee042 100644
--- a/lib/lp/snappy/tests/test_snapbuildbehaviour.py
+++ b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test snap package build behaviour."""
@@ -57,6 +57,7 @@ from lp.buildmaster.enums import (
     BuildBaseImageType,
     BuildStatus,
     )
+from lp.buildmaster.interactor import shut_down_default_threadpool
 from lp.buildmaster.interfaces.builder import CannotBuild
 from lp.buildmaster.interfaces.buildfarmjobbehaviour import (
     IBuildFarmJobBehaviour,
@@ -314,7 +315,7 @@ class TestAsyncSnapBuildBehaviour(TestSnapBuildBehaviourBase):
         builder.processor = job.build.processor
         slave = self.useFixture(SlaveTestHelpers()).getClientSlave()
         job.setBuilder(builder, slave)
-        self.addCleanup(slave.pool.closeCachedConnections)
+        self.addCleanup(shut_down_default_threadpool)
         return job
 
     @defer.inlineCallbacks
@@ -356,7 +357,7 @@ class TestAsyncSnapBuildBehaviour(TestSnapBuildBehaviourBase):
                         Equals(b"Basic " + base64.b64encode(
                             b"admin-launchpad.test:admin-secret"))]),
                     b"Content-Type": MatchesListwise([
-                        Equals(b"application/json; charset=UTF-8"),
+                        Equals(b"application/json"),
                         ]),
                     }),
                 "content": AfterPreprocessing(json.loads, MatchesDict({