golang/session: initial implementation and basic test

[progrium]

This is the meat of the implementation. Although I'd like better test coverage and comments, I'm particularly concerned about the use of contexts. I don't believe they are used where given or appropriate where given so I'd like to think about them more.

Also, I don't often centralize errors into predefined variables, but it might be a good idea to here?

Double check min-max limits ... there are packet sizes and data packet sizes and I'm not sure they're correctly used?

After this there is simply a transport subpackage with various transport protocol dialers and such.

If synchronous review works better, schedule a time with me!

[shazow]

Looks plausible! Definitely very ssh-esque.

Also is auth going to be part of qmux at all, or are we assuming a pre-authed io stream?

One other scenario that's worth keeping in mind is somekind of graceful reload support when possible, without losing state/connections. That's one thing that's particularly difficult with ssh.

[mgood]

I'm still reading, but also curious about the context

[progrium]

Also is auth going to be part of qmux at all, or are we assuming a pre-authed io stream?

Auth is out of scope. Either it's handled in the underlying transport (WebSocket, for example), preceding the session (TLS client certs maybe), or in a protocol above this (maybe you're muxing HTTP, or maybe you have an RPC auth mechanism).

One other scenario that's worth keeping in mind is somekind of graceful reload support when possible, without losing state/connections. That's one thing that's particularly difficult with ssh.

That's hard in general right? What are some examples?

[shazow]

That's hard in general right? What are some examples?

Yea. There's lots of implementations for this in HTTP, but HTTP is fairly stateless so it's less hard. Here's a fairly general TCP implementation: https://github.com/facebookarchive/grace/blob/75cf19382434/gracenet/net.go#L206

This is probably another transport-level thing but it helps when the protocol/API doesn't get in the way.

[progrium]

@mgood @shazow besides removing stored contexts, and adding context to Open, I've also removed the Listener interface (for now) and dropped the LocalAddr/RemoteAddr methods from Session, since if Session is on a net.Conn, you should be able to type assert as net.Conn to get those. They don't make sense if over stdio for example.

[shazow]

That does look better.

[progrium]

I'll take that as a pass for now!