From 6a7067e4b6fbcd3ffa5112c388e1ceee914fa336 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Fri, 3 Dec 2021 18:45:43 +0530 Subject: [PATCH] Clarify comment on golang-migrate connection (#19104) The old comment wasn't exactly clear on why is it needed to spawn a new connection. ```release-note NONE ``` --- store/sqlstore/store.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/store/sqlstore/store.go b/store/sqlstore/store.go index d39cb3bbe9..b27fe5a4ae 100644 --- a/store/sqlstore/store.go +++ b/store/sqlstore/store.go @@ -1429,8 +1429,11 @@ func (ss *SqlStore) migrate(direction migrationDirection) error { var driver database.Driver var err error - // When WithInstance is used in golang-migrate, the underlying driver connections are not tracked. - // So we will have to open a fresh connection for migrations and explicitly close it when all is done. + // golang-migrate doesn't have a way to reuse an existing connection in a migration. + // The current API will always close the current connection _as well as_ the *sql.DB + // instance along with it. Which means that we cannot pass an existing DB instance, + // because it will be closed. Therefore, we always have to create a new instance, + // and therefore a new connection. dataSource, err := ss.appendMultipleStatementsFlag(*ss.settings.DataSource) if err != nil { return err