Launchpad itself

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

  1. Git
  2. lp:~cjwatson/launchpad
  3. charmhub-upload-fix-check-status-url
  4. Merge into 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
diff --git a/lib/lp/charms/model/charmhubclient.py b/lib/lp/charms/model/charmhubclient.py
index a376097..112a73e 100644
--- a/lib/lp/charms/model/charmhubclient.py
+++ b/lib/lp/charms/model/charmhubclient.py
@@ -223,6 +223,8 @@ class CharmhubClient:
223 @classmethod223 @classmethod
224 def checkStatus(cls, build, status_url):224 def checkStatus(cls, build, status_url):
225 """See `ICharmhubClient`."""225 """See `ICharmhubClient`."""
226 status_url = urlappend(
227 config.charms.charmhub_url, status_url.lstrip("/"))
226 macaroon_raw = _get_macaroon(build.recipe)228 macaroon_raw = _get_macaroon(build.recipe)
227 request = get_current_browser_request()229 request = get_current_browser_request()
228 timeline_action = get_request_timeline(request).start(230 timeline_action = get_request_timeline(request).start(
diff --git a/lib/lp/charms/tests/test_charmhubclient.py b/lib/lp/charms/tests/test_charmhubclient.py
index ce61daa..176b14c 100644
--- a/lib/lp/charms/tests/test_charmhubclient.py
+++ b/lib/lp/charms/tests/test_charmhubclient.py
@@ -156,8 +156,8 @@ class TestCharmhubClient(TestCaseWithFactory):
156 status=200,156 status=200,
157 json={157 json={
158 "status-url": (158 "status-url": (
159 "http://charmhub.example/v1/charm/{}/revisions/review"159 "/v1/charm/{}/revisions/review?upload-id=123".format(
160 "?upload-id=123".format(quote(name))),160 quote(name))),
161 })161 })
162162
163 def _addCharmReleaseResponse(self, name):163 def _addCharmReleaseResponse(self, name):
@@ -311,8 +311,7 @@ class TestCharmhubClient(TestCaseWithFactory):
311 # config.ICharmhubUploadJobSource.dbuser once that job exists.311 # config.ICharmhubUploadJobSource.dbuser once that job exists.
312 with dbuser("charm-build-job"):312 with dbuser("charm-build-job"):
313 self.assertEqual(313 self.assertEqual(
314 "http://charmhub.example/v1/charm/test-charm/revisions/review"314 "/v1/charm/test-charm/revisions/review?upload-id=123",
315 "?upload-id=123",
316 self.client.upload(build))315 self.client.upload(build))
317 requests = [call.request for call in responses.calls]316 requests = [call.request for call in responses.calls]
318 self.assertThat(requests, MatchesListwise([317 self.assertThat(requests, MatchesListwise([
@@ -380,11 +379,9 @@ class TestCharmhubClient(TestCaseWithFactory):
380 def test_checkStatus_pending(self):379 def test_checkStatus_pending(self):
381 self._setUpSecretStorage()380 self._setUpSecretStorage()
382 build = self.makeUploadableCharmRecipeBuild()381 build = self.makeUploadableCharmRecipeBuild()
383 status_url = (382 status_url = "/v1/charm/test-charm/revisions/review?upload-id=123"
384 "http://charmhub.example/v1/charm/test-charm/revisions/review"
385 "?upload-id=123")
386 responses.add(383 responses.add(
387 "GET", status_url,384 "GET", "http://charmhub.example" + status_url,
388 json={385 json={
389 "revisions": [386 "revisions": [
390 {387 {
@@ -403,11 +400,9 @@ class TestCharmhubClient(TestCaseWithFactory):
403 def test_checkStatus_error(self):400 def test_checkStatus_error(self):
404 self._setUpSecretStorage()401 self._setUpSecretStorage()
405 build = self.makeUploadableCharmRecipeBuild()402 build = self.makeUploadableCharmRecipeBuild()
406 status_url = (403 status_url = "/v1/charm/test-charm/revisions/review?upload-id=123"
407 "http://charmhub.example/v1/charm/test-charm/revisions/review"
408 "?upload-id=123")
409 responses.add(404 responses.add(
410 "GET", status_url,405 "GET", "http://charmhub.example" + status_url,
411 json={406 json={
412 "revisions": [407 "revisions": [
413 {408 {
@@ -428,11 +423,9 @@ class TestCharmhubClient(TestCaseWithFactory):
428 def test_checkStatus_approved_no_revision(self):423 def test_checkStatus_approved_no_revision(self):
429 self._setUpSecretStorage()424 self._setUpSecretStorage()
430 build = self.makeUploadableCharmRecipeBuild()425 build = self.makeUploadableCharmRecipeBuild()
431 status_url = (426 status_url = "/v1/charm/test-charm/revisions/review?upload-id=123"
432 "http://charmhub.example/v1/charm/test-charm/revisions/review"
433 "?upload-id=123")
434 responses.add(427 responses.add(
435 "GET", status_url,428 "GET", "http://charmhub.example" + status_url,
436 json={429 json={
437 "revisions": [430 "revisions": [
438 {431 {
@@ -452,11 +445,9 @@ class TestCharmhubClient(TestCaseWithFactory):
452 def test_checkStatus_approved(self):445 def test_checkStatus_approved(self):
453 self._setUpSecretStorage()446 self._setUpSecretStorage()
454 build = self.makeUploadableCharmRecipeBuild()447 build = self.makeUploadableCharmRecipeBuild()
455 status_url = (448 status_url = "/v1/charm/test-charm/revisions/review?upload-id=123"
456 "http://charmhub.example/v1/charm/test-charm/revisions/review"
457 "?upload-id=123")
458 responses.add(449 responses.add(
459 "GET", status_url,450 "GET", "http://charmhub.example" + status_url,
460 json={451 json={
461 "revisions": [452 "revisions": [
462 {453 {
@@ -471,7 +462,7 @@ class TestCharmhubClient(TestCaseWithFactory):
471 requests = [call.request for call in responses.calls]462 requests = [call.request for call in responses.calls]
472 self.assertThat(requests, MatchesListwise([463 self.assertThat(requests, MatchesListwise([
473 RequestMatches(464 RequestMatches(
474 url=Equals(status_url),465 url=Equals("http://charmhub.example" + status_url),
475 method=Equals("GET"),466 method=Equals("GET"),
476 auth=("Macaroon", MacaroonVerifies(self.exchanged_key))),467 auth=("Macaroon", MacaroonVerifies(self.exchanged_key))),
477 ]))468 ]))
@@ -480,10 +471,9 @@ class TestCharmhubClient(TestCaseWithFactory):
480 def test_checkStatus_404(self):471 def test_checkStatus_404(self):
481 self._setUpSecretStorage()472 self._setUpSecretStorage()
482 build = self.makeUploadableCharmRecipeBuild()473 build = self.makeUploadableCharmRecipeBuild()
483 status_url = (474 status_url = "/v1/charm/test-charm/revisions/review?upload-id=123"
484 "http://charmhub.example/v1/charm/test-charm/revisions/review"475 responses.add(
485 "?upload-id=123")476 "GET", "http://charmhub.example" + status_url, status=404)
486 responses.add("GET", status_url, status=404)
487 self.assertRaisesWithContent(477 self.assertRaisesWithContent(
488 BadReviewStatusResponse, "404 Client Error: Not Found",478 BadReviewStatusResponse, "404 Client Error: Not Found",
489 self.client.checkStatus, build, status_url)479 self.client.checkStatus, build, status_url)
diff --git a/lib/lp/charms/tests/test_charmrecipebuildjob.py b/lib/lp/charms/tests/test_charmrecipebuildjob.py
index 6c7aaa9..9bc020d 100644
--- a/lib/lp/charms/tests/test_charmrecipebuildjob.py
+++ b/lib/lp/charms/tests/test_charmrecipebuildjob.py
@@ -110,9 +110,7 @@ class TestCharmhubUploadJob(TestCaseWithFactory):
110 CHARM_RECIPE_ALLOW_CREATE: "on",110 CHARM_RECIPE_ALLOW_CREATE: "on",
111 CHARM_RECIPE_WEBHOOKS_FEATURE_FLAG: "on",111 CHARM_RECIPE_WEBHOOKS_FEATURE_FLAG: "on",
112 }))112 }))
113 self.status_url = (113 self.status_url = "/v1/charm/test-charm/revisions/review?upload-id=123"
114 "http://charmhub.example/v1/charm/test-charm/revisions/review"
115 "?upload-id=123")
116114
117 def test_provides_interface(self):115 def test_provides_interface(self):
118 # `CharmhubUploadJob` objects provide `ICharmhubUploadJob`.116 # `CharmhubUploadJob` objects provide `ICharmhubUploadJob`.

Subscribers

People subscribed via source and target branches

to status/vote changes: