Merge ~cjwatson/launchpad:charmhub-upload-fix-check-status-url into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: adb4747d5038baa145d815a462858d5f35040ea5
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:charmhub-upload-fix-check-status-url
Merge into: launchpad:master
Diff against target: 132 lines (+18/-28)
3 files modified
lib/lp/charms/model/charmhubclient.py (+2/-0)
lib/lp/charms/tests/test_charmhubclient.py (+15/-25)
lib/lp/charms/tests/test_charmrecipebuildjob.py (+1/-3)
Reviewer Review Type Date Requested Status
Cristian Gonzalez (community) Approve
Review via email: mp+408182@code.launchpad.net

Commit message

Fix up URL in CharmhubClient.checkStatus

Description of the change

Charmhub's `/v1/charm/:name/revisions` endpoint in fact returns a *relative* URL, not an absolute one. Fix `CharmhubClient.checkStatus` to make it absolute so that we can actually make requests to it.

To post a comment you must log in.
Revision history for this message
Cristian Gonzalez (cristiangsp) wrote :

Looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/charms/model/charmhubclient.py b/lib/lp/charms/model/charmhubclient.py
2index a376097..112a73e 100644
3--- a/lib/lp/charms/model/charmhubclient.py
4+++ b/lib/lp/charms/model/charmhubclient.py
5@@ -223,6 +223,8 @@ class CharmhubClient:
6 @classmethod
7 def checkStatus(cls, build, status_url):
8 """See `ICharmhubClient`."""
9+ status_url = urlappend(
10+ config.charms.charmhub_url, status_url.lstrip("/"))
11 macaroon_raw = _get_macaroon(build.recipe)
12 request = get_current_browser_request()
13 timeline_action = get_request_timeline(request).start(
14diff --git a/lib/lp/charms/tests/test_charmhubclient.py b/lib/lp/charms/tests/test_charmhubclient.py
15index ce61daa..176b14c 100644
16--- a/lib/lp/charms/tests/test_charmhubclient.py
17+++ b/lib/lp/charms/tests/test_charmhubclient.py
18@@ -156,8 +156,8 @@ class TestCharmhubClient(TestCaseWithFactory):
19 status=200,
20 json={
21 "status-url": (
22- "http://charmhub.example/v1/charm/{}/revisions/review"
23- "?upload-id=123".format(quote(name))),
24+ "/v1/charm/{}/revisions/review?upload-id=123".format(
25+ quote(name))),
26 })
27
28 def _addCharmReleaseResponse(self, name):
29@@ -311,8 +311,7 @@ class TestCharmhubClient(TestCaseWithFactory):
30 # config.ICharmhubUploadJobSource.dbuser once that job exists.
31 with dbuser("charm-build-job"):
32 self.assertEqual(
33- "http://charmhub.example/v1/charm/test-charm/revisions/review"
34- "?upload-id=123",
35+ "/v1/charm/test-charm/revisions/review?upload-id=123",
36 self.client.upload(build))
37 requests = [call.request for call in responses.calls]
38 self.assertThat(requests, MatchesListwise([
39@@ -380,11 +379,9 @@ class TestCharmhubClient(TestCaseWithFactory):
40 def test_checkStatus_pending(self):
41 self._setUpSecretStorage()
42 build = self.makeUploadableCharmRecipeBuild()
43- status_url = (
44- "http://charmhub.example/v1/charm/test-charm/revisions/review"
45- "?upload-id=123")
46+ status_url = "/v1/charm/test-charm/revisions/review?upload-id=123"
47 responses.add(
48- "GET", status_url,
49+ "GET", "http://charmhub.example" + status_url,
50 json={
51 "revisions": [
52 {
53@@ -403,11 +400,9 @@ class TestCharmhubClient(TestCaseWithFactory):
54 def test_checkStatus_error(self):
55 self._setUpSecretStorage()
56 build = self.makeUploadableCharmRecipeBuild()
57- status_url = (
58- "http://charmhub.example/v1/charm/test-charm/revisions/review"
59- "?upload-id=123")
60+ status_url = "/v1/charm/test-charm/revisions/review?upload-id=123"
61 responses.add(
62- "GET", status_url,
63+ "GET", "http://charmhub.example" + status_url,
64 json={
65 "revisions": [
66 {
67@@ -428,11 +423,9 @@ class TestCharmhubClient(TestCaseWithFactory):
68 def test_checkStatus_approved_no_revision(self):
69 self._setUpSecretStorage()
70 build = self.makeUploadableCharmRecipeBuild()
71- status_url = (
72- "http://charmhub.example/v1/charm/test-charm/revisions/review"
73- "?upload-id=123")
74+ status_url = "/v1/charm/test-charm/revisions/review?upload-id=123"
75 responses.add(
76- "GET", status_url,
77+ "GET", "http://charmhub.example" + status_url,
78 json={
79 "revisions": [
80 {
81@@ -452,11 +445,9 @@ class TestCharmhubClient(TestCaseWithFactory):
82 def test_checkStatus_approved(self):
83 self._setUpSecretStorage()
84 build = self.makeUploadableCharmRecipeBuild()
85- status_url = (
86- "http://charmhub.example/v1/charm/test-charm/revisions/review"
87- "?upload-id=123")
88+ status_url = "/v1/charm/test-charm/revisions/review?upload-id=123"
89 responses.add(
90- "GET", status_url,
91+ "GET", "http://charmhub.example" + status_url,
92 json={
93 "revisions": [
94 {
95@@ -471,7 +462,7 @@ class TestCharmhubClient(TestCaseWithFactory):
96 requests = [call.request for call in responses.calls]
97 self.assertThat(requests, MatchesListwise([
98 RequestMatches(
99- url=Equals(status_url),
100+ url=Equals("http://charmhub.example" + status_url),
101 method=Equals("GET"),
102 auth=("Macaroon", MacaroonVerifies(self.exchanged_key))),
103 ]))
104@@ -480,10 +471,9 @@ class TestCharmhubClient(TestCaseWithFactory):
105 def test_checkStatus_404(self):
106 self._setUpSecretStorage()
107 build = self.makeUploadableCharmRecipeBuild()
108- status_url = (
109- "http://charmhub.example/v1/charm/test-charm/revisions/review"
110- "?upload-id=123")
111- responses.add("GET", status_url, status=404)
112+ status_url = "/v1/charm/test-charm/revisions/review?upload-id=123"
113+ responses.add(
114+ "GET", "http://charmhub.example" + status_url, status=404)
115 self.assertRaisesWithContent(
116 BadReviewStatusResponse, "404 Client Error: Not Found",
117 self.client.checkStatus, build, status_url)
118diff --git a/lib/lp/charms/tests/test_charmrecipebuildjob.py b/lib/lp/charms/tests/test_charmrecipebuildjob.py
119index 6c7aaa9..9bc020d 100644
120--- a/lib/lp/charms/tests/test_charmrecipebuildjob.py
121+++ b/lib/lp/charms/tests/test_charmrecipebuildjob.py
122@@ -110,9 +110,7 @@ class TestCharmhubUploadJob(TestCaseWithFactory):
123 CHARM_RECIPE_ALLOW_CREATE: "on",
124 CHARM_RECIPE_WEBHOOKS_FEATURE_FLAG: "on",
125 }))
126- self.status_url = (
127- "http://charmhub.example/v1/charm/test-charm/revisions/review"
128- "?upload-id=123")
129+ self.status_url = "/v1/charm/test-charm/revisions/review?upload-id=123"
130
131 def test_provides_interface(self):
132 # `CharmhubUploadJob` objects provide `ICharmhubUploadJob`.

Subscribers

People subscribed via source and target branches

to status/vote changes: