Merge ~cjwatson/launchpad:py3-timelimitedtoken into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 028ef5d74fc1d5c00b6ea679bd61b79f18f7e2ba
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:py3-timelimitedtoken
Merge into: launchpad:master
Diff against target: 53 lines (+7/-5)
2 files modified
lib/lp/services/librarian/model.py (+5/-4)
lib/lp/services/librarianserver/tests/test_web.py (+2/-1)
Reviewer Review Type Date Requested Status
Cristian Gonzalez (community) Approve
Review via email: mp+398917@code.launchpad.net

Commit message

Fix TimeLimitedToken for Python 3

Description of the change

TimeLimitedToken.allocate needs to return a native string, not bytes.

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/services/librarian/model.py b/lib/lp/services/librarian/model.py
2index 0cd870e..d2b3d0f 100644
3--- a/lib/lp/services/librarian/model.py
4+++ b/lib/lp/services/librarian/model.py
5@@ -16,6 +16,7 @@ import hashlib
6
7 from lazr.delegates import delegate_to
8 import pytz
9+import six
10 from six.moves.urllib.parse import urlparse
11 from sqlobject import (
12 BoolCol,
13@@ -309,7 +310,7 @@ class TimeLimitedToken(StormBase):
14 def allocate(url):
15 """Allocate a token for url path in the librarian.
16
17- :param url: A url bytestring. e.g.
18+ :param url: A url string. e.g.
19 https://i123.restricted.launchpad-librarian.net/123/foo.txt
20 Note that the token is generated for 123/foo.txt
21 :return: A url fragment token ready to be attached to the url.
22@@ -317,14 +318,14 @@ class TimeLimitedToken(StormBase):
23 """
24 store = session_store()
25 path = TimeLimitedToken.url_to_token_path(url)
26- token = create_token(32).encode('ascii')
27- store.add(TimeLimitedToken(path, token))
28+ token = create_token(32)
29+ store.add(TimeLimitedToken(path, token.encode('ascii')))
30 # The session isn't part of the main transaction model, and in fact it
31 # has autocommit on. The commit here is belts and bracers: after
32 # allocation the external librarian must be able to serve the file
33 # immediately.
34 store.commit()
35- return token
36+ return six.ensure_str(token)
37
38 @staticmethod
39 def url_to_token_path(url):
40diff --git a/lib/lp/services/librarianserver/tests/test_web.py b/lib/lp/services/librarianserver/tests/test_web.py
41index 5133aa5..4366f07 100644
42--- a/lib/lp/services/librarianserver/tests/test_web.py
43+++ b/lib/lp/services/librarianserver/tests/test_web.py
44@@ -443,7 +443,8 @@ class LibrarianWebTestCase(LibrarianWebTestMixin, TestCaseWithFactory):
45 store = session_store()
46 tokens = store.find(
47 TimeLimitedToken,
48- TimeLimitedToken.token == hashlib.sha256(token).hexdigest())
49+ TimeLimitedToken.token == hashlib.sha256(
50+ token.encode('ASCII')).hexdigest())
51 tokens.set(
52 TimeLimitedToken.created == SQL("created - interval '1 week'"))
53 # Now, as per test_restricted_no_token we should get a 404.

Subscribers

People subscribed via source and target branches

to status/vote changes: