Umzug's CLI API lacks consideration for DB connection lifecycle #622
Unanswered
Hornwitser
asked this question in
General
Replies: 1 comment
-
It's worth noting a couple of things:
That said, I agree that in practice your use case is common, and umzug considers sequelize a first-class citizen, so it would be nice to have something either built in, or easy to configure and well-documented for this scenario. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
In Sequelize you create a database connection with
And tear it down with
And until the connection has been torn down it may hold timers and connections in the event loop, and thefore keep the Node.js application running for however long those timers and connections take to time out.
Despite this being how most software talking to a database work Umzug's CLI API seems to just completely ignore this? The examples using Sequelize don't call
.close()
. The most sensible option I found to implement closing the database after being done with it is to export aclose
function in the definition of the umzug migrationsand then call this after runAsCLI
Another pain point is that to create an
Umzug
migrator with aSequelizeStorage
adapter it needs to be passed aSequelize
instance which in turn needs a valid database URI. But if you are a user of this tool and want to see the--help
text, getting an error telling you you're missing the database URI might make you think there is no--help
and that passing the database credentials will make it execute the migrations.The CLI should not need a database connection to show its
--help
text, and it should have a sensible place for tearing down the database connection after it's done with it. Exactly how these things are to be implemented I don't have a good idea for. ThebeforeCommand
andafterCommand
are a candidate as they don't get called when passing--help
, but currently there's no way to set the storage and context from them.Beta Was this translation helpful? Give feedback.
All reactions