Merge lp://staging/~mterry/ubuntuone-couch/extra-headers into lp://staging/ubuntuone-couch
Proposed by
Michael Terry
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | dobey | ||||
Approved revision: | 9 | ||||
Merged at revision: | 8 | ||||
Proposed branch: | lp://staging/~mterry/ubuntuone-couch/extra-headers | ||||
Merge into: | lp://staging/ubuntuone-couch | ||||
Diff against target: |
22 lines (+4/-2) 1 file modified
ubuntuone/couch/auth.py (+4/-2) |
||||
To merge this branch: | bzr merge lp://staging/~mterry/ubuntuone-couch/extra-headers | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
dobey (community) | Approve | ||
Eric Casteleijn (community) | Approve | ||
Review via email: mp+61648@code.staging.launchpad.net |
Commit message
Allow extra headers in auth.request()
To post a comment you must log in.
Looks good, except this is a bad idea:
+ token_secret=None, headers={}):
As default values in Python should as a rule not be mutable types. Default values are evaluated at import time, making this empty dictionary a 'global' variable, so that in this case the oauth headers for one call to request() will pollute the dictionary for any subsequent calls.
Please change it to:
+ token_secret=None, headers=None):
and then at the beginning of the function implementation do:
headers = headers or {}
Also a test case would be awesome, but if you cannot think of one, I can look at that.