Launchpad itself

Merge ~jelmer/launchpad:public-https into launchpad:master

  1. Git
  2. lp:~jelmer/launchpad
  3. public-https
  4. Merge into master
Proposed by Jelmer Vernooij
Status: Needs review
Proposed branch: ~jelmer/launchpad:public-https
Merge into: launchpad:master
Diff against target: 76 lines (+9/-10)
4 files modified
lib/lp/code/interfaces/branch.py (+2/-2)
lib/lp/code/interfaces/codehosting.py (+1/-1)
lib/lp/code/model/branch.py (+2/-2)
lib/lp/code/tests/test_branch.py (+4/-5)
Reviewer Review Type Date Requested Status
Launchpad code reviewers Pending
Review via email: mp+435608@code.launchpad.net

Commit message

Add https as a supported scheme for public branches

Description of the change

Add https as a supported scheme for public branches

To post a comment you must log in.
Revision history for this message
Jürgen Gmach (jugmac00) wrote :

Could you please re-trigger CI? There seems to be some lxd issue, which - fingers-crossed - is temporary.

Reply
Revision history for this message
Colin Watson (cjwatson) wrote :

Hmm, is this a good idea right now? Compare:

  $ brz branch http://bazaar.launchpad.net/~brz/brz/trunk brz
  # works

  $ brz branch https://bazaar.launchpad.net/~brz/brz/trunk brz
  https://bazaar.launchpad.net/~brz/brz/trunk/ is permanently redirected to https://bazaar.launchpad.net/~brz/brz/trunk/changes/
  brz: ERROR: Unexpected HTTP status 500 for https://bazaar.launchpad.net/~brz/brz/trunk/changes/info/refs?service=git-upload-pack: Unable to handle http code: Internal Server Error

I'm not quite sure whose problem that is - do you know if it's yours or ours? (When I curl those two URLs, I seem to get a similar redirect in each case, which suggests a Breezy bug, but I'm not sure.)

Reply
Revision history for this message
Jelmer Vernooij (jelmer) wrote (last edit ):

Maybe a bit of both..

I think it's a launchpad (loggerhead?) issue that the 500 is served - it should be a 404. Although it's triggered by breezy trying to fetch a URL which should not exist (it tries to see if there is a working tree).

E.g. trying to retrieve https://bazaar.launchpad.net/~brz/brz/trunk/changes/info results in a 500

The https URL also seems to redirect to http under the hood .

Reply
Revision history for this message
Colin Watson (cjwatson) wrote :

True, but the http:// version of that also 500s, so why does this only cause a failure to branch for https://?

Reply
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

> True, but the http:// version of that also 500s, so why does this only cause a
> failure to branch for https://?

We seem to be unable to find a bzr branch there, so fall back to probing for a git repository - which leads to the 500.

Confusingly https://code.launchpad.net/brz (as opposed to https://code.launchpad.net/~brz/brz/trunk), which is what I originally tested before I submitted this MR, works fine.

Also of note:

$ curl https://code.launchpad.net/~brz/brz/trunk/.bzr/branch/location
http://bazaar.launchpad.net/~brz/brz/trunk

Reply
Revision history for this message
Guruprasad (lgp171188) wrote :

On lunar, a command like 'bzr branch lp:lptools' has been failing with an error like 'brz: ERROR: Not a branch: "git+ssh://<email address hidden>/lptools": Repository 'lptools' not found..'. I have reported a bug for it - https://bugs.launchpad.net/ubuntu/+source/breezy/+bug/2023998. Colin mentioned that it could be related to https://bugs.launchpad.net/launchpad/+bug/2002568 and https://code.launchpad.net/~jelmer/launchpad/+git/launchpad/+merge/435608.

Reply
Revision history for this message
Colin Watson (cjwatson) wrote (last edit ):

There was a recent thread on ubuntu-devel pointing out why `lp:` fails and providing a workaround (`lp+bzr:`), so that part may not be LP's fault: https://lists.ubuntu.com/archives/ubuntu-devel/2023-June/042619.html

Reply

Unmerged commits

4b523e4... by Jelmer Vernooij

Support https URLs for public branches.

Fixes: https://launchpad.net/bugs/2002569

Succeeded
[SUCCEEDED] docs:0 (build)
[SUCCEEDED] lint:0 (build)
[SUCCEEDED] mypy:0 (build)
1 → 3 of 3 results First • Previous • Next • Last

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/code/interfaces/branch.py b/lib/lp/code/interfaces/branch.py
2index d579132..657ed5d 100644
3--- a/lib/lp/code/interfaces/branch.py
4+++ b/lib/lp/code/interfaces/branch.py
5@@ -277,11 +277,11 @@ class IBranchView(
6 id = Int(title=_("ID"), readonly=True, required=True)
7
8 @operation_parameters(
9- scheme=TextLine(title=_("URL scheme"), default="http")
10+ scheme=TextLine(title=_("URL scheme"), default="https")
11 )
12 @export_read_operation()
13 @operation_for_version("beta")
14- def composePublicURL(scheme="http"):
15+ def composePublicURL(scheme="https"):
16 """Return a public URL for the branch using the given protocol.
17
18 :param scheme: a protocol name accepted by the public
19diff --git a/lib/lp/code/interfaces/codehosting.py b/lib/lp/code/interfaces/codehosting.py
20index 6c3be3a..86c1fa1 100644
21--- a/lib/lp/code/interfaces/codehosting.py
22+++ b/lib/lp/code/interfaces/codehosting.py
23@@ -68,7 +68,7 @@ def branch_id_alias(branch):
24
25
26 # The scheme types that are supported for codehosting.
27-SUPPORTED_SCHEMES = "bzr+ssh", "http"
28+SUPPORTED_SCHEMES = "bzr+ssh", "http", "https"
29
30
31 class IBazaarApplication(ILaunchpadApplication):
32diff --git a/lib/lp/code/model/branch.py b/lib/lp/code/model/branch.py
33index 71c7fc9..0dcc350 100644
34--- a/lib/lp/code/model/branch.py
35+++ b/lib/lp/code/model/branch.py
36@@ -810,10 +810,10 @@ class Branch(SQLBase, WebhookTargetMixin, BzrIdentityMixin):
37 def browse_source_url(self):
38 return self.getCodebrowseUrl("files")
39
40- def composePublicURL(self, scheme="http"):
41+ def composePublicURL(self, scheme="https"):
42 """See `IBranch`."""
43 # Not all protocols work for private branches.
44- public_schemes = ["http"]
45+ public_schemes = ["https", "http"]
46 assert not (self.private and scheme in public_schemes), (
47 "Private branch %s has no public URL." % self.unique_name
48 )
49diff --git a/lib/lp/code/tests/test_branch.py b/lib/lp/code/tests/test_branch.py
50index 065f8cd..6c7f1be 100644
51--- a/lib/lp/code/tests/test_branch.py
52+++ b/lib/lp/code/tests/test_branch.py
53@@ -379,10 +379,10 @@ class TestComposePublicURL(TestCaseWithFactory):
54 sftp_url = branch.composePublicURL("sftp")
55 self.assertEqual(url_pattern % "sftp", sftp_url)
56
57- def test_composePublicURL_default_http(self):
58+ def test_composePublicURL_default_https(self):
59 # The default scheme for composePublicURL is http.
60 branch = self.factory.makeAnyBranch()
61- prefix = "http://"
62+ prefix = "https://"
63 public_url = branch.composePublicURL()
64 self.assertEqual(prefix, public_url[: len(prefix)])
65
66@@ -398,8 +398,7 @@ class TestComposePublicURL(TestCaseWithFactory):
67 )
68 self.assertRaises(AssertionError, branch.composePublicURL, "http")
69
70- def test_composePublicURL_no_https(self):
71- # There's no https support. If there were, it should probably
72- # not work for private branches.
73+ def test_composePublicURL_https_private(self):
74+ # Private branches don't have public https URLs.
75 branch = self.factory.makeAnyBranch()
76 self.assertRaises(AssertionError, branch.composePublicURL, "https")

Subscribers

People subscribed via source and target branches

to status/vote changes: