Add before_disconnect callback to Connection #2
Conversation
Added a simple callback to Connection. This callback will be called from the `disconnect/2` function on
MyXQL.Connection, enabling users to do extra stuff just before the connection is disconnected (for instance
send a COM_QUIT as per the MySQL connection conventions).
The callback gets the state of the Connection as its only argument. Part
of this state is `state.client`, which can be used to send the
aforementioned COM_QUIT:
in `config/dev.exs`
```elixir
config :my_app, MyApp.Repo,
username: "...",
port: 3306,
before_disconnect_callback: fn state ->
MyXQL.Client.com_quit(state.client)
end
```
This approach where we use a callback allows for opt-in from developers
and does not change any behavior unless a callback is actually implemented.
| @impl true | ||
| def disconnect(_reason, state) do | ||
| if state.before_disconnect_callback do | ||
| state.before_disconnect_callback.(state) |
There was a problem hiding this comment.
Two questions:
- I think this is a blocking call, is it worth exploring making the callback call async?
- Should we be rescue'ing here in case of errors?
There was a problem hiding this comment.
I think it's not an issue the callback call itself is synchronous; if a user wants it to be asynchronous they are still able to make it so in the actual implementation of their callback.
For this case I think it's ok to have it synchronous as I think we want to want to make sure the callback is run before we do the actual disconnect of the client.
2). Yeah, we probably should if we make this an actual PR 🤔
|
Can we try and contribute this upstream so we don't end up depending on another internal fork? |
If this turns out to actually improve things I think we should contribute it to upstream; It would probably need some improvements before that though (exception handling, tests). It's not my intention to keep it as a fork for the long term, however, I do not want to spend too much time on it before we know for sure it actually improves things. So for me this is more like a "let's try this" instead of a "I'm sure this works and should become part of the library". |
Added a simple callback to Connection. This callback will be called from the
disconnect/2function on MyXQL.Connection, enabling users to do extra stuff just before the connection is disconnected (for instance send a COM_QUIT as per the MySQL connection conventions).The callback gets the state of the Connection as its only argument. Part of this state is
state.client, which can be used to send the aforementioned COM_QUIT:in
config/dev.exsThis approach where we use a callback allows for opt-in from developers and does not change any behavior unless a callback is actually implemented.