mysql: Add ConnectionReady callback to Handler interface #9496
Conversation
This is is useful to determine the intermediate state after NewConnection and before ConnectionClosed, basically for a handler to know if authentication, etc, were successful. NewConnection is way too soon in the process to determine if the connection got fully established. Signed-off-by: Matt Robenolt <matt@ydekproductions.com>
mattrobenolt
requested review from
frouioui,
harshit-gangal and
systay
as code owners
| // Tell our handler that we're finished handshake and are ready to | ||
| // process commands. | ||
| l.handler.ConnectionReady(c) |
There was a problem hiding this comment.
I'm open to where we think it's best to determine Ready. It seemed to me to put it right before the for{} loop that started reading commands. But wasn't sure if yall had an opinion on putting it before the SlowConnectionWarnThreshold check, since in theory this callback can add extra connection time. I at least felt it was important to put after the last OK packet sent.
| @@ -820,6 +820,10 @@ func (t testRun) NewConnection(c *Conn) { | |||
| panic("implement me") | |||
| } | |||
|
|
|||
| func (t testRun) ConnectionReady(c *Conn) { | |||
There was a problem hiding this comment.
This is just filling in all of the other internal implementations of the Handler interface.
When doing this, I was thinking it might be nice to introduce a stub UnimplementedHandler struct that all implementation can embed that provides default no-op handlers. Similar to how gRPC gives you the stub to satisfy the interface. That way an implementation isn't require to implement every callback if they're not needed.
I'd be happy to follow up with an UnimplementedHandler struct if you also think this is valuable. It'd help wiht the boilerplate in the tests and all that get touched in this PR.
frouioui
added
release notes
Type: Enhancement
|
@frouioui correct, within vitess core, there's no use for it. It's a no-op hook so other implementations of |
This is is useful to determine the intermediate state after
NewConnection and before ConnectionClosed, basically for a handler to
know if authentication, etc, were successful. NewConnection is way too
soon in the process to determine if the connection got fully
established.
Signed-off-by: Matt Robenolt matt@ydekproductions.com
Checklist