Closed ieugen closed 2 years ago
@yogthos : All tests pass for this. I ported just a few functions but I think most will follow the pattern with connection-or-spec (see above).
@yogthos : In this context modify-sql-fn
receives an vector which is not as documented in the README - where the function expects a string.
(sql-mig/do-commands
t-con
(modify-sql-fn
[(str "ALTER TABLE " table-name " ADD COLUMN description varchar(1024)")
(str "ALTER TABLE " table-name " ADD COLUMN applied timestamp")]))
p.s. Migrated this to jdbc/execute-batch!
So I tested this with PostgreSQL migration from the work project and got some failures in the way configuration is handled. Started a discussion on Slack and posting here to have issue history:
looking at the code I think the feature with :connection and :datasource was broken for some time.: Database gets the config with :db {:connection xxx }
(defrecord Database [connection config] proto/Store (config [this] config) (init [this] (let [conn (connect* (assoc (:db config) :transaction? (:init-in-transaction? config)))]
then in connect* we have (had):
(defn connect* [db] (let [^Connection conn (cond (instance? Connection db) db (instance? DataSource db) (try (.getConnection ^DataSource db) (catch Exception e (log/error e (str "Error getting DB connection from source" db)))) :else (try (sql/get-connection db) (catch Exception e (log/error e (str "Error creating DB connection for " (utils/censor-password db))))))]
connect* checks the db var with instance? , not (:connection db) as is documented in README things in docs changed at this point https://github.com/yogthos/migratus/pull/183/files
what is the way to go here?
Yup, that looks like a bug. Probably would make sense to assume there is a :connection
key going forward, and then do update
on it?
PR for #182
There are some differences in how java.jdbc works and how next.jdbc works and I don't know what is the best way to handle this in migratus. migratus receives a db-spec and builds a connection that it uses as :connection . Most functions use this connection via this pattern:
java.jdbc
has a functiondb-find-connection
that extracts the connection from the map. There is also logic to create a connection if one is not found.next.jdbc
does not have this so I implemented: