Issue 7374044: Try and re-authorise automatically

Issue 7374044: Try and re-authorise automatically (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
11 years, 2 months ago by wallyworld

Modified:
11 years, 2 months ago

Reviewers:
dimitern, dave, mp+149473

Visibility:
Public.

Description

Try and re-authorise automatically If a request returns 401 or 403, attempt to re-authorise and then retry the request. The service double hook code was moved to a new package so that some tests could be written without circular import issues. https://code.launchpad.net/~wallyworld/goose/auto-reauth/+merge/149473 Requires: https://code.launchpad.net/~wallyworld/goose/single-http-client/+merge/148970 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 9

Created: 11 years, 2 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+316 lines, -205 lines)			Patch
A	[revision details]	View	1 chunk	+2 lines, -0 lines	0 comments	Download
M	client/client.go	View	2 chunks	+12 lines, -1 line	2 comments	Download
M	errors/errors.go	View	3 chunks	+16 lines, -0 lines	3 comments	Download
M	errors/errors_test.go	View	1 chunk	+14 lines, -0 lines	0 comments	Download
M	http/client.go	View	1 chunk	+4 lines, -0 lines	2 comments	Download
M	nova/local_test.go	View	5 chunks	+41 lines, -4 lines	0 comments	Download
A	testservices/hook/service.go	View	1 chunk	+86 lines, -0 lines	0 comments	Download
A	testservices/hook/service_test.go	View	1 chunk	+105 lines, -0 lines	0 comments	Download
M	testservices/identityservice/userpass.go	View	3 chunks	+23 lines, -10 lines	2 comments	Download
M	testservices/novaservice/service.go	View	1 chunk	+0 lines, -1 line	0 comments	Download
M	testservices/service.go	View	3 chunks	+2 lines, -73 lines	0 comments	Download
D	testservices/service_test.go	View	1 chunk	+0 lines, -105 lines	0 comments	Download
M	tools/secgroup-delete-all/main_test.go	View	4 chunks	+11 lines, -11 lines	0 comments	Download

Messages

Total messages: 4

Expand All Messages | Collapse All Messages

dimitern

LGTM with some questions. https://codereview.appspot.com/7374044/diff/1/errors/errors.go File errors/errors.go (right): https://codereview.appspot.com/7374044/diff/1/errors/errors.go#newcode17 errors/errors.go:17: UnauthorisedError = Code("Unauthenticted") "Unauthorised" maybe? ...

11 years, 2 months ago (2013-02-20 07:51:15 UTC) #2

dave_cheney.net

LGTM modulo the comment about adding c.setToken() https://codereview.appspot.com/7374044/diff/1/client/client.go File client/client.go (right): https://codereview.appspot.com/7374044/diff/1/client/client.go#newcode136 client/client.go:136: c.mu.Lock() I ...

11 years, 2 months ago (2013-02-21 02:00:00 UTC) #3

wallyworld

11 years, 2 months ago (2013-02-21 02:43:54 UTC) #4

https://codereview.appspot.com/7374044/diff/1/client/client.go
File client/client.go (right):

https://codereview.appspot.com/7374044/diff/1/client/client.go#newcode136
client/client.go:136: c.mu.Lock()
On 2013/02/21 02:00:00, dfc wrote:
> I think you need a c.setToken(string) method to capture all this mutex locking
> and unlocking.

Done.

https://codereview.appspot.com/7374044/diff/1/errors/errors.go
File errors/errors.go (right):

https://codereview.appspot.com/7374044/diff/1/errors/errors.go#newcode17
errors/errors.go:17: UnauthorisedError   = Code("Unauthenticted")
On 2013/02/20 07:51:15, dimitern wrote:
> "Unauthorised" maybe?

Thanks, I changed the wording and missed this bit.
Done.

https://codereview.appspot.com/7374044/diff/1/http/client.go
File http/client.go (right):

https://codereview.appspot.com/7374044/diff/1/http/client.go#newcode266
http/client.go:266: case http.StatusNotFound:
On 2013/02/21 02:00:00, dfc wrote:
> Please remove the braces

Done.

https://codereview.appspot.com/7374044/diff/1/testservices/identityservice/us...
File testservices/identityservice/userpass.go (right):

https://codereview.appspot.com/7374044/diff/1/testservices/identityservice/us...
testservices/identityservice/userpass.go:237: // XXX: We should really build up
valid state for this instead, at the
On 2013/02/20 07:51:15, dimitern wrote:
> this still valid? i though we fixed the service urls / regions.

AFAIK, the comment refers to the method of generating the test response, by
parsing a string copied from a sample response and poking new values into it.

Expand All Messages | Collapse All Messages