contexts #29
contexts #29
Conversation
… to go with it Signed-off-by: Jeff Lindsay <progrium@gmail.com>
|
cc @belak |
context.go
Outdated
| } | ||
| // for most of these, instead of converting to strings now, storing the byte | ||
| // slices means allocations only happen when accessing, not when contexts | ||
| // are being copied around |
There was a problem hiding this comment.
Though after I saw how context.WithValue is implemented this might not be an issue.
session.go
Outdated
| @@ -43,6 +44,10 @@ type Session interface { | |||
| // used it will return nil. | |||
| PublicKey() PublicKey | |||
|
|
|||
| Context() context.Context | |||
There was a problem hiding this comment.
Would it be worthwhile ensuring there are methods for everything exported by the ssh.Context? Especially if we're returning this as a context.Context here.
There was a problem hiding this comment.
Sigh. The problem is the Session interface is already sooooo huuuuuge and it's only going to get bigger with signals and I also added Permissions. Interfaces should be small. A lot of the rest of what's in ssh.Context does qualify as "to inform, not to control" and is more for maintainers than users (at least how I interpret it). So it might feel inconsistent, but at the moment I'm ok with the tradeoff.
Signed-off-by: Jeff Lindsay <progrium@gmail.com>
Signed-off-by: Jeff Lindsay <progrium@gmail.com>
Signed-off-by: Jeff Lindsay <progrium@gmail.com>
|
Actually I think it's pretty much done with godocs and enough testing for now. Would like some more reviews first though. |
|
@shazow thanks! |
|
LGTM |
|
Merged after running in (alpha) production in Cmd.io for 20 days. |
Implements a special ssh.Context that captures gossh.ConnMetadata and context.Context made for better callbacks and session setup, then exposed as regular context.Context on ssh.Session
Standardizes callbacks more while giving them access to more information and letting them contribute to state that would be made available to Session handler. Removes the need for PermissionsCallback as Permissions are exposed on ssh.Context. Generally simplifies code overall, while increasing capabilities, and remaining fairly Go idiomatic.
Todo: godocs, more tests