yogthos / migratus

MIGRATE ALL THE THINGS!
641 stars 95 forks source link

Allow managing the connection state from calling code #255

Closed b-ryan closed 8 months ago

b-ryan commented 8 months ago

Discussion:

https://github.com/yogthos/migratus/issues/253#issuecomment-1819840435

https://github.com/yogthos/migratus/issues/253#issuecomment-1819854502

I wonder if the implementation will be easier by allowing the db map to look like:

{:connection ... :managed-connection? true}

If so it looks to me like the feature could be achieved by modifying migratus.core/run to:

(defn run [store ids command]
  (try
    (log/info "Starting migrations")
    (proto/connect store)
    (command store ids)
    (catch java.sql.BatchUpdateException e
      (throw (or (.getNextException e) e)))
    (finally
      (log/info "Ending migrations")
      (when-not (:managed-connection? (:db (:config store))) (proto/disconnect store)))))

Dunno if this is how you'd want this implemented or whether it covers every case though.

b-ryan commented 8 months ago

Maybe the robust way would be to have a separate implementation of Store that does nothing inside disconnect.

yogthos commented 8 months ago

Yeah, I think that would be cleaner. The store implementation could be selected based on the config, and then everything else should work same as before.