Code review comment for lp:~leonardr/launchpad/automatically-calculate-request-token-expire-time

37 @@ -254,10 +257,19 @@
38 else:
39 return None
40
41 + @property
42 + def is_expired(self):
43 + now = datetime.now(pytz.timezone('UTC'))
44 + expires = self.date_created + timedelta(hours=REQUEST_TOKEN_VALIDITY)
45 + return expires <= now
46 +
47 def review(self, user, permission, context=None):
48 """See `IOAuthRequestToken`."""
49 assert not self.is_reviewed, (
50 "Request tokens can be reviewed only once.")
51 + assert not self.is_expired, (
52 + 'This request token has expired and can no longer be reviewed.'
53 + )
54 self.date_reviewed = datetime.now(pytz.timezone('UTC'))
55 self.person = user
56 self.permission = permission
57 @@ -279,6 +291,11 @@
58 'Cannot create an access token from an unreviewed request token.')
59 assert self.permission != OAuthPermission.UNAUTHORIZED, (
60 'The user did not grant access to this consumer.')
61 + assert not self.is_expired, (
62 + 'This request token has expired and can no longer be exchanged '
63 + 'for an access token.'
64 + )
65 +

I think it's better to raise AssertionError with your text than it is to use the assert command. At least, that's the policy we've been trying to enforce.

Other than that, I think this branch is good.