Merge ~cjwatson/launchpad:fix-buildmaster-fetch-in-thread into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: fb59dcfe74532aeb061a018f94196df97d06c86b
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:fix-buildmaster-fetch-in-thread
Merge into: launchpad:master
Diff against target: 139 lines (+24/-16)
4 files modified
lib/lp/buildmaster/interactor.py (+1/-0)
lib/lp/oci/tests/test_ocirecipebuildbehaviour.py (+6/-3)
lib/lp/snappy/model/snapbuildbehaviour.py (+13/-10)
lib/lp/snappy/tests/test_snapbuildbehaviour.py (+4/-3)
Reviewer Review Type Date Requested Status
Tom Wardill (community) Approve
Review via email: mp+383734@code.launchpad.net

Commit message

Fix proxy token handling for buildd-manager changes

Description of the change

Now that buildd-manager fetches files using threads, both the code used to fetch snap proxy tokens and its associated tests need some adjustments.

To post a comment you must log in.
Revision history for this message
Tom Wardill (twom) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/buildmaster/interactor.py b/lib/lp/buildmaster/interactor.py
2index e711862..64a54f3 100644
3--- a/lib/lp/buildmaster/interactor.py
4+++ b/lib/lp/buildmaster/interactor.py
5@@ -6,6 +6,7 @@ __metaclass__ = type
6 __all__ = [
7 'BuilderInteractor',
8 'extract_vitals_from_db',
9+ 'shut_down_default_threadpool',
10 ]
11
12 from collections import namedtuple
13diff --git a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
14index ad09ea4..8536072 100644
15--- a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
16+++ b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
17@@ -41,7 +41,10 @@ from lp.buildmaster.enums import (
18 BuildBaseImageType,
19 BuildStatus,
20 )
21-from lp.buildmaster.interactor import BuilderInteractor
22+from lp.buildmaster.interactor import (
23+ BuilderInteractor,
24+ shut_down_default_threadpool,
25+ )
26 from lp.buildmaster.interfaces.builder import (
27 BuildDaemonError,
28 CannotBuild,
29@@ -111,7 +114,7 @@ class MakeOCIBuildMixin:
30 builder.processor = job.build.processor
31 slave = self.useFixture(SlaveTestHelpers()).getClientSlave()
32 job.setBuilder(builder, slave)
33- self.addCleanup(slave.pool.closeCachedConnections)
34+ self.addCleanup(shut_down_default_threadpool)
35
36 # Taken from test_archivedependencies.py
37 for component_name in ("main", "universe"):
38@@ -207,7 +210,7 @@ class TestAsyncOCIRecipeBuildBehaviour(MakeOCIBuildMixin, TestCaseWithFactory):
39 Equals(b"Basic " + base64.b64encode(
40 b"admin-launchpad.test:admin-secret"))]),
41 b"Content-Type": MatchesListwise([
42- Equals(b"application/json; charset=UTF-8"),
43+ Equals(b"application/json"),
44 ]),
45 }),
46 "content": AfterPreprocessing(json.loads, MatchesDict({
47diff --git a/lib/lp/snappy/model/snapbuildbehaviour.py b/lib/lp/snappy/model/snapbuildbehaviour.py
48index cbcba1d..c34534e 100644
49--- a/lib/lp/snappy/model/snapbuildbehaviour.py
50+++ b/lib/lp/snappy/model/snapbuildbehaviour.py
51@@ -1,4 +1,4 @@
52-# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
53+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
54 # GNU Affero General Public License version 3 (see the file LICENSE).
55
56 """An `IBuildFarmJobBehaviour` for `SnapBuild`.
57@@ -17,12 +17,15 @@ __all__ = [
58 import base64
59 import time
60
61+from requests import Session
62 from six.moves.urllib.parse import (
63 urlsplit,
64 urlunsplit,
65 )
66-import treq
67-from twisted.internet import defer
68+from twisted.internet import (
69+ defer,
70+ threads,
71+ )
72 from zope.component import adapter
73 from zope.interface import implementer
74 from zope.security.proxy import removeSecurityProxy
75@@ -39,7 +42,6 @@ from lp.registry.interfaces.series import SeriesStatus
76 from lp.services.config import config
77 from lp.services.features import getFeatureFlag
78 from lp.services.twistedsupport import cancel_on_timeout
79-from lp.services.twistedsupport.treq import check_status
80 from lp.snappy.interfaces.snap import (
81 SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG,
82 SnapBuildArchiveOwnerMismatch,
83@@ -101,13 +103,14 @@ class SnapProxyMixin:
84 auth_string = '{}:{}'.format(admin_username, secret).strip()
85 auth_header = b'Basic ' + base64.b64encode(auth_string)
86
87- response = yield treq.post(
88+ session = Session()
89+ session.trust_env = False
90+ response = yield threads.deferToThreadPool(
91+ self._slave.reactor, self._slave.threadpool, session.post,
92 url, headers={'Authorization': auth_header},
93- json={'username': proxy_username},
94- reactor=self._slave.reactor,
95- pool=self._slave.pool)
96- response = yield check_status(response)
97- token = yield treq.json_content(response)
98+ json={'username': proxy_username})
99+ response.raise_for_status()
100+ token = response.json()
101 defer.returnValue(token)
102
103
104diff --git a/lib/lp/snappy/tests/test_snapbuildbehaviour.py b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
105index f851008..dfee042 100644
106--- a/lib/lp/snappy/tests/test_snapbuildbehaviour.py
107+++ b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
108@@ -1,4 +1,4 @@
109-# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
110+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
111 # GNU Affero General Public License version 3 (see the file LICENSE).
112
113 """Test snap package build behaviour."""
114@@ -57,6 +57,7 @@ from lp.buildmaster.enums import (
115 BuildBaseImageType,
116 BuildStatus,
117 )
118+from lp.buildmaster.interactor import shut_down_default_threadpool
119 from lp.buildmaster.interfaces.builder import CannotBuild
120 from lp.buildmaster.interfaces.buildfarmjobbehaviour import (
121 IBuildFarmJobBehaviour,
122@@ -314,7 +315,7 @@ class TestAsyncSnapBuildBehaviour(TestSnapBuildBehaviourBase):
123 builder.processor = job.build.processor
124 slave = self.useFixture(SlaveTestHelpers()).getClientSlave()
125 job.setBuilder(builder, slave)
126- self.addCleanup(slave.pool.closeCachedConnections)
127+ self.addCleanup(shut_down_default_threadpool)
128 return job
129
130 @defer.inlineCallbacks
131@@ -356,7 +357,7 @@ class TestAsyncSnapBuildBehaviour(TestSnapBuildBehaviourBase):
132 Equals(b"Basic " + base64.b64encode(
133 b"admin-launchpad.test:admin-secret"))]),
134 b"Content-Type": MatchesListwise([
135- Equals(b"application/json; charset=UTF-8"),
136+ Equals(b"application/json"),
137 ]),
138 }),
139 "content": AfterPreprocessing(json.loads, MatchesDict({

Subscribers

People subscribed via source and target branches

to status/vote changes: