Merge ~pelpsi/launchpad:fetch-service-revocation-endpoint-fix into launchpad:master

Proposed by Simone Pelosi
Status: Merged
Approved by: Simone Pelosi
Approved revision: bb3d32f4a9e736135a18baa8884ad41802aaa9f9
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~pelpsi/launchpad:fetch-service-revocation-endpoint-fix
Merge into: launchpad:master
Diff against target: 29 lines (+3/-3)
2 files modified
lib/lp/buildmaster/builderproxy.py (+2/-2)
lib/lp/buildmaster/tests/fetchservice.py (+1/-1)
Reviewer Review Type Date Requested Status
Ines Almeida Approve
Review via email: mp+462260@code.launchpad.net

Commit message

Fix revocation endpoint

fetch_service_control_endpoint has the following structure:
http://{host}:{port}/session
Fix revocation_endpoint from http://{host}:{port}/session/session to
http://{host}:{port}/session

To post a comment you must log in.
Revision history for this message
Ines Almeida (ines-almeida) wrote :

Can you be more explicit in the commit message about that this is about the fetch service revocation endpoint?

Also, maybe more of a personal preference, but I would write the structure as something like `http://{host}:{port}/{session}` (with some sort of brackets). As it is, you could mean something like `http://0.0.0.0:80/session` because it's not as clear.

review: Approve
Revision history for this message
Simone Pelosi (pelpsi) wrote :

Thank you for the review! I updated the commit message and changed the from id to session_id

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 75e049f..3375042 100644
3--- a/lib/lp/buildmaster/builderproxy.py
4+++ b/lib/lp/buildmaster/builderproxy.py
5@@ -77,9 +77,9 @@ class BuilderProxyMixin:
6 port=_get_proxy_config("fetch_service_port"),
7 )
8 )
9- args["revocation_endpoint"] = "{endpoint}/session/{id}".format(
10+ args["revocation_endpoint"] = "{endpoint}/{session_id}".format(
11 endpoint=_get_proxy_config("fetch_service_control_endpoint"),
12- id=session["id"],
13+ session_id=session["id"],
14 )
15
16 @defer.inlineCallbacks
17diff --git a/lib/lp/buildmaster/tests/fetchservice.py b/lib/lp/buildmaster/tests/fetchservice.py
18index ea70651..dd21e27 100644
19--- a/lib/lp/buildmaster/tests/fetchservice.py
20+++ b/lib/lp/buildmaster/tests/fetchservice.py
21@@ -117,7 +117,7 @@ class RevocationEndpointMatcher(Equals):
22
23 def __init__(self, session_id):
24 super().__init__(
25- "{endpoint}/session/{session_id}".format(
26+ "{endpoint}/{session_id}".format(
27 endpoint=config.builddmaster.fetch_service_control_endpoint,
28 session_id=session_id,
29 )

Subscribers

People subscribed via source and target branches

to status/vote changes: