Launchpad itself

Merge ~ines-almeida/launchpad:fetch-service-buildd-manager-refactor into launchpad:master

  1. Git
  2. lp:~ines-almeida/launchpad
  3. fetch-service-buildd-manager-refactor
  4. Merge into master
Proposed by Ines Almeida
Status: Merged
Approved by: Ines Almeida
Approved revision: 77953d20c0d4e7a7b5e49aefda025cd2a51da0bd
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~ines-almeida/launchpad:fetch-service-buildd-manager-refactor
Merge into: launchpad:master
Diff against target: 186 lines (+16/-26)
5 files modified
lib/lp/buildmaster/builderproxy.py (+4/-10)
lib/lp/buildmaster/downloader.py (+2/-5)
lib/lp/buildmaster/interactor.py (+1/-0)
lib/lp/buildmaster/tests/fetchservice.py (+8/-6)
lib/lp/snappy/tests/test_snapbuildbehaviour.py (+1/-5)
Reviewer Review Type Date Requested Status
Jürgen Gmach Approve
Review via email: mp+464684@code.launchpad.net

Commit message

Refactor buildd-manager: remove unneeded variables, rename and add comments

 - Removed the `/session` from the base URL for the fetch service control API endpoint, for consistency.
 - The `proxy_username` was confirmed to not be necessary for the fetch service.
 - Minor comment and variable refactoring

To post a comment you must log in.
Revision history for this message
Jürgen Gmach (jugmac00) :
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/builderproxy.py b/lib/lp/buildmaster/builderproxy.py
2index a2b8f71..304c258 100644
3--- a/lib/lp/buildmaster/builderproxy.py
4+++ b/lib/lp/buildmaster/builderproxy.py
5@@ -58,7 +58,7 @@ class BuilderProxyMixin:
6 use_fetch_service: bool = False,
7 ) -> Generator[None, Dict[str, str], None]:
8
9- self.proxy_service = None
10+ self._proxy_service = None
11
12 if not allow_internet:
13 return
14@@ -80,10 +80,10 @@ class BuilderProxyMixin:
15 # non-production environments.
16 return
17
18- self.proxy_service = proxy_service(
19+ self._proxy_service = proxy_service(
20 build_id=self.build.build_cookie, worker=self._worker
21 )
22- new_session = yield self.proxy_service.startSession()
23+ new_session = yield self._proxy_service.startSession()
24 args["proxy_url"] = new_session["proxy_url"]
25 args["revocation_endpoint"] = new_session["revocation_endpoint"]
26
27@@ -181,7 +181,7 @@ class FetchService(IProxyService):
28 """
29
30 PROXY_URL = "http://{session_id}:{token}@{proxy_endpoint}"
31- START_SESSION_ENDPOINT = "{control_endpoint}"
32+ START_SESSION_ENDPOINT = "{control_endpoint}/session"
33 TOKEN_REVOCATION = "{control_endpoint}/session/{session_id}/token"
34 RETRIEVE_METADATA_ENDPOINT = "{control_endpoint}/session/{session_id}"
35 END_SESSION_ENDPOINT = "{control_endpoint}/session/{session_id}"
36@@ -217,18 +217,12 @@ class FetchService(IProxyService):
37
38 See IProxyService.
39 """
40- timestamp = int(time.time())
41- proxy_username = "{build_id}-{timestamp}".format(
42- build_id=self.build_id, timestamp=timestamp
43- )
44-
45 session_data = yield self.worker.process_pool.doWork(
46 RequestFetchServiceSessionCommand,
47 url=self.START_SESSION_ENDPOINT.format(
48 control_endpoint=self.control_endpoint
49 ),
50 auth_header=self.auth_header,
51- proxy_username=proxy_username,
52 )
53
54 self.session_id = session_data["id"]
55diff --git a/lib/lp/buildmaster/downloader.py b/lib/lp/buildmaster/downloader.py
56index 7cf1c9b..ba3eaf2 100644
57--- a/lib/lp/buildmaster/downloader.py
58+++ b/lib/lp/buildmaster/downloader.py
59@@ -49,7 +49,6 @@ class RequestFetchServiceSessionCommand(amp.Command):
60 arguments = [
61 (b"url", amp.Unicode()),
62 (b"auth_header", amp.String()),
63- (b"proxy_username", amp.Unicode()),
64 ]
65 response = [
66 (b"id", amp.Unicode()),
67@@ -119,9 +118,7 @@ class RequestProcess(AMPChild):
68 return response.json()
69
70 @RequestFetchServiceSessionCommand.responder
71- def requestFetchServiceSessionCommand(
72- self, url, auth_header, proxy_username
73- ):
74+ def requestFetchServiceSessionCommand(self, url, auth_header):
75 with Session() as session:
76 session.trust_env = False
77 # XXX pelpsi: from ST108 and from what Claudio
78@@ -134,7 +131,7 @@ class RequestProcess(AMPChild):
79 response = session.post(
80 url,
81 headers={"Authorization": auth_header},
82- json={"username": proxy_username, "policy": "permissive"},
83+ json={"policy": "permissive"},
84 )
85 response.raise_for_status()
86 return response.json()
87diff --git a/lib/lp/buildmaster/interactor.py b/lib/lp/buildmaster/interactor.py
88index 1ba8b7b..dfcc86b 100644
89--- a/lib/lp/buildmaster/interactor.py
90+++ b/lib/lp/buildmaster/interactor.py
91@@ -3,6 +3,7 @@
92
93 __all__ = [
94 "BuilderInteractor",
95+ "BuilderWorker",
96 "extract_vitals_from_db",
97 ]
98
99diff --git a/lib/lp/buildmaster/tests/fetchservice.py b/lib/lp/buildmaster/tests/fetchservice.py
100index 51f73e3..34b6122 100644
101--- a/lib/lp/buildmaster/tests/fetchservice.py
102+++ b/lib/lp/buildmaster/tests/fetchservice.py
103@@ -17,16 +17,18 @@ from twisted.web import resource, server
104 from lp.services.config import config
105
106
107-class FetchServiceAuthAPITokensResource(resource.Resource):
108- """A test session resource for the fetch service authentication API."""
109+class FetchServiceControlEndpoint(resource.Resource):
110+ """A fake fetch service control endpoints API."""
111
112 isLeaf = True
113
114 def __init__(self):
115 resource.Resource.__init__(self)
116 self.requests = []
117+ self.session_id = "2ea9c9f759044f9b9aff469fe90429a5"
118
119 def render_POST(self, request):
120+ """We make a POST request to start a fetch service session"""
121 content = json.loads(request.content.read())
122 self.requests.append(
123 {
124@@ -38,7 +40,7 @@ class FetchServiceAuthAPITokensResource(resource.Resource):
125 )
126 return json.dumps(
127 {
128- "id": "1",
129+ "id": self.session_id,
130 "token": uuid.uuid4().hex,
131 }
132 ).encode("UTF-8")
133@@ -68,7 +70,7 @@ class InProcessFetchServiceAuthAPIFixture(fixtures.Fixture):
134 @defer.inlineCallbacks
135 def start(self):
136 root = resource.Resource()
137- self.sessions = FetchServiceAuthAPITokensResource()
138+ self.sessions = FetchServiceControlEndpoint()
139 root.putChild(b"sessions", self.sessions)
140 endpoint = endpoints.serverFromString(reactor, nativeString("tcp:0"))
141 site = server.Site(self.sessions)
142@@ -80,7 +82,7 @@ class InProcessFetchServiceAuthAPIFixture(fixtures.Fixture):
143 [builddmaster]
144 fetch_service_control_admin_secret: admin-secret
145 fetch_service_control_admin_username: admin-launchpad.test
146- fetch_service_control_endpoint: http://{host}:{port}/session
147+ fetch_service_control_endpoint: http://{host}:{port}
148 fetch_service_host: {host}
149 fetch_service_port: {port}
150 fetch_service_mitm_certificate: fake-cert
151@@ -116,7 +118,7 @@ class RevocationEndpointMatcher(Equals):
152
153 def __init__(self, session_id):
154 super().__init__(
155- "{endpoint}/{session_id}".format(
156+ "{endpoint}/session/{session_id}/token".format(
157 endpoint=config.builddmaster.fetch_service_control_endpoint,
158 session_id=session_id,
159 )
160diff --git a/lib/lp/snappy/tests/test_snapbuildbehaviour.py b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
161index d20bf33..781643c 100644
162--- a/lib/lp/snappy/tests/test_snapbuildbehaviour.py
163+++ b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
164@@ -361,13 +361,10 @@ class TestAsyncSnapBuildBehaviourFetchService(
165 request = self.factory.makeSnapBuildRequest(snap=snap)
166 job = self.makeJob(snap=snap, build_request=request)
167 args = yield job.extraBuildArgs()
168- expected_uri = urlsplit(
169- config.builddmaster.fetch_service_control_endpoint
170- ).path.encode("UTF-8")
171 request_matcher = MatchesDict(
172 {
173 "method": Equals(b"POST"),
174- "uri": Equals(expected_uri),
175+ "uri": Equals(b"/session"),
176 "headers": ContainsDict(
177 {
178 b"Authorization": MatchesListwise(
179@@ -389,7 +386,6 @@ class TestAsyncSnapBuildBehaviourFetchService(
180 ),
181 "json": MatchesDict(
182 {
183- "username": StartsWith(job.build.build_cookie + "-"),
184 "policy": Equals("permissive"),
185 }
186 ),

Subscribers

People subscribed via source and target branches

to status/vote changes: