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
diff --git a/lib/lp/services/librarian/model.py b/lib/lp/services/librarian/model.py
index 0cd870e..d2b3d0f 100644
--- a/lib/lp/services/librarian/model.py
+++ b/lib/lp/services/librarian/model.py
@@ -16,6 +16,7 @@ import hashlib
1616
17from lazr.delegates import delegate_to17from lazr.delegates import delegate_to
18import pytz18import pytz
19import six
19from six.moves.urllib.parse import urlparse20from six.moves.urllib.parse import urlparse
20from sqlobject import (21from sqlobject import (
21 BoolCol,22 BoolCol,
@@ -309,7 +310,7 @@ class TimeLimitedToken(StormBase):
309 def allocate(url):310 def allocate(url):
310 """Allocate a token for url path in the librarian.311 """Allocate a token for url path in the librarian.
311312
312 :param url: A url bytestring. e.g.313 :param url: A url string. e.g.
313 https://i123.restricted.launchpad-librarian.net/123/foo.txt314 https://i123.restricted.launchpad-librarian.net/123/foo.txt
314 Note that the token is generated for 123/foo.txt315 Note that the token is generated for 123/foo.txt
315 :return: A url fragment token ready to be attached to the url.316 :return: A url fragment token ready to be attached to the url.
@@ -317,14 +318,14 @@ class TimeLimitedToken(StormBase):
317 """318 """
318 store = session_store()319 store = session_store()
319 path = TimeLimitedToken.url_to_token_path(url)320 path = TimeLimitedToken.url_to_token_path(url)
320 token = create_token(32).encode('ascii')321 token = create_token(32)
321 store.add(TimeLimitedToken(path, token))322 store.add(TimeLimitedToken(path, token.encode('ascii')))
322 # The session isn't part of the main transaction model, and in fact it323 # The session isn't part of the main transaction model, and in fact it
323 # has autocommit on. The commit here is belts and bracers: after324 # has autocommit on. The commit here is belts and bracers: after
324 # allocation the external librarian must be able to serve the file325 # allocation the external librarian must be able to serve the file
325 # immediately.326 # immediately.
326 store.commit()327 store.commit()
327 return token328 return six.ensure_str(token)
328329
329 @staticmethod330 @staticmethod
330 def url_to_token_path(url):331 def url_to_token_path(url):
diff --git a/lib/lp/services/librarianserver/tests/test_web.py b/lib/lp/services/librarianserver/tests/test_web.py
index 5133aa5..4366f07 100644
--- a/lib/lp/services/librarianserver/tests/test_web.py
+++ b/lib/lp/services/librarianserver/tests/test_web.py
@@ -443,7 +443,8 @@ class LibrarianWebTestCase(LibrarianWebTestMixin, TestCaseWithFactory):
443 store = session_store()443 store = session_store()
444 tokens = store.find(444 tokens = store.find(
445 TimeLimitedToken,445 TimeLimitedToken,
446 TimeLimitedToken.token == hashlib.sha256(token).hexdigest())446 TimeLimitedToken.token == hashlib.sha256(
447 token.encode('ASCII')).hexdigest())
447 tokens.set(448 tokens.set(
448 TimeLimitedToken.created == SQL("created - interval '1 week'"))449 TimeLimitedToken.created == SQL("created - interval '1 week'"))
449 # Now, as per test_restricted_no_token we should get a 404.450 # Now, as per test_restricted_no_token we should get a 404.

Subscribers

People subscribed via source and target branches

to status/vote changes: