signalwire
WIP: First pass at ignoring signals
I'm not a huge fan of this solution... ignoring signals solves the problem directly, but is kind of an odd solution, relying on pushing signal handling to the end user for a fairly common scenario of having to clean up or wait for call completion.
On startup we have setup (pre-connect) and ready (post-connect)
On shutdown we only have teardown (post-disconnect)... what we're missing is a pre-disconnect callback method.
I would suggest instead we expose another callback method (name TBD) that allows the user to manage their business logic while shutting down but before its disconnected from Relay.
The result I think is the same, clients would write the same code, but it would be in a SDK supported callback rather than hacking signal handling.
That would be a great solution, though we are going to eventually redo all SDKs. Also, adding a consumer interface method makes it so all SDKs have to support it. I think this is a good stopgap measure.
On Fri, 20 Nov 2020 at 20:12, Bryan Rite [email protected] wrote:
I'm not a huge fan of this solution... ignoring signals solves the problem for Replicant directly, but is kind of an odd solution, relying on pushing signal handling to the end user for a fairly common scenario of having to clean up or wait for call completion.
On startup we have setup (pre-connect) and ready (post-connect)
On shutdown we only have teardown (post-disconnect)... what we're missing is a pre-disconnect callback method.
I would suggest instead we expose another callback method (name TBD) that allows the user to manage their business logic while shutting down but before its disconnected from Relay.
The result I think is the same, Replicant would write the same code, but it would be in a SDK supported callback rather than hacking signal handling.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/signalwire/signalwire-node/pull/271#issuecomment-731357774, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB2TSV5PHLRGZFXQZSBRWTSQ25SXANCNFSM4T356XAA .
Not really, we should eventually have it for all of them, but we can add it just for Node right now, it's just as much if not less code than this... rather than having a client rely on something we won't support in the future.
Can you take an attempt at it? Would just have to follow whatever is done for teardown, setup, or ready, and probably hook it into gracefulShutdown.