bakery: remove Request type. #12
Conversation
| @@ -19,6 +19,36 @@ import ( | |||
| "gopkg.in/macaroon-bakery.v0/bakery" | |||
| ) | |||
|
|
|||
| // DefaultHTTPClient is an http.Client that ensures that | |||
There was a problem hiding this comment.
Is it a go bug that the default client doesn't do this?
There was a problem hiding this comment.
Added a link to the bug.
|
👍 LGTM |
| @@ -33,6 +37,7 @@ func DischargeAll( | |||
| if err != nil { | |||
| return nil, errgo.NoteMask(err, fmt.Sprintf("cannot get discharge from %q", cav.Location), errgo.Any) | |||
| } | |||
| dm.Bind(sig) | |||
There was a problem hiding this comment.
I can't think of a reason you wouldn't want to bind when discharging all the macaroons, but this behavior should at least be documented in the godoc comment.
Would binding twice break things? I have a feeling it would, I'd have to go look at the impl of Macaroon.Bind.
There was a problem hiding this comment.
It is already documented in the godoc doc: "All the discharge macaroons
will be bound to the primary macaroon."
Yes, binding twice would break things. But I don't think that should
be an issue - you can't usefully add caveats to a macaroon after
discharging it, so I think that doing the binding here makes sense.
If it turns out it's actually useful to be able to discharge but not bind,
we can always create a DischargeAllUnbound function in the future.
I'm hoping that no-one will need to call Bind directly any more.
|
LGTM with the bulk errors in CheckRequest addressed here or in a followup. @alesstimec note the API changes in this PR. Thanks! |
|
@cmars Noted. Will change, as soon as this PR lands. |
httpbakery: remove Service type. Add cookie helpers.
rogpeppe
force-pushed
the
008-lose-request
branch
from
98e922e
to
52bfba3
Compare
httpbakery: remove Service type. Add cookie helpers.
Also change bakery.DischargeAll so that it returns a slice suitable
for putting in a cookie.