tconbeer / harlequin

The SQL IDE for Your Terminal.
https://harlequin.sh
MIT License
3.37k stars 75 forks source link

feat: close the adapter's connection on quit #518

Closed mpetazzoni closed 3 months ago

mpetazzoni commented 3 months ago

This change introduces a new optional method of HarlequinConnection implementations in adapters, giving them an opportunity to cleanly close the connection to the underlying database when the app exists.

The close() method is called in the action_quit() handler.

Does this PR require a change to Harlequin's docs?

Did you add or update tests for this change?

Please complete the following checklist:

tconbeer commented 3 months ago

Thanks. Can I ask what prompted this? I thought most python DB libraries close the connection when the handle leaves scope/is deleted.

mpetazzoni commented 3 months ago

@tconbeer Thanks for taking a look! Python does not guarantee that the __del__ finalizer gets called on interpreter exit, and since the Harlequin app keeps a reference to it (self.connection), nor does it use a context manager for the connection object around the lifecycle of the Harlequin app, there is no current possible mechanism for an adapter to perform a graceful shutdown/close of its connection to the database.

Of course, connections are severed when Python exists, but there might situations where adapters want to perform on-shutdown actions (flush, commit, emit telemetry, or gracefully release resources). This new close() method gives them an opportunity to do so, if they want to. See for example the DB-API spec.

tconbeer commented 3 months ago

ok, I'm on-board. thanks for the extra info, makes sense.