Code review comment for lp:~leonardr/launchpadlib/improve-workflow

Revision history for this message
Martin Pool (mbp) wrote :

+ self.output(self.WAITING_FOR_USER % self.authorization_url)
+ while credentials.access_token is None:
+ try:
+ credentials.exchange_request_token_for_access_token(web_root)
+ except HTTPError, e:
+ if e.response.status == 401:
+ # The user has not made their decision yet.
+ time.sleep(1)
+ elif e.response.status == 403:
+ # The user decided not to authorize this
+ # application.
+ raise EndUserDeclinedAuthorization(e.content)
+ else:
+ # The user decided to grant access.
+ # credentials.access_token is no longer None.
+ break

This treats a 500 error as deciding to grant access. Really? I think you want the 'break' inside the try block. Aside from that it looks good.

 review: tweak

I do wonder a bit about the load this will introduce but perhaps we can measure that. Can we find out from the logs how many times this is called at the moment?

review: Needs Fixing

« Back to merge proposal