yogthos / migratus

MIGRATE ALL THE THINGS!
640 stars 95 forks source link

Add option to list ALL migrations considered by migratus #240

Open ieugen opened 1 year ago

ieugen commented 1 year ago

I think it would be very useful to have a way to list the migration ids that migratus knows about (maybe with file path?) Migrations up / down can use the ID's but to my knowledge there is no easy way to print them. I'm using clj-migratus, but I think this should be part of core and exposed in clj-migratus.

Searching the filesystem will not necessarily give me the migrations that migratus discovers / considers .

Brought this up on the list as well: https://clojurians.slack.com/archives/C1Q164V29/p1686770183348569

ieugen commented 1 year ago

Feedback from Sean Corfield

I can definitely see value in a migrations library being able to produce a list of all possible migrations, indicating which have been applied an which haven't (if the system is able to track on a per-migration basis whether it has been applied or not). Our hand-rolled system at work is "linear" (it has a monotonically-increased "version" number and just tracks the highest version applied) and if I was starting over again, I'd use Migratus and I would love the "report" feature that provided more information, especially if it could list the date a migration was applied, etc.

ieugen commented 1 year ago

pending list does not work quite as expected IMO. It lists the migration name but not the ID.

bb migrate pending-list
[articles-publish-idx]

So trying to up that will fail. I propose we add / updated pending-list to display also the ID's ?!

 bb migrate up articles-publish-idx 
{:clojure.main/message
 "Execution error (NumberFormatException) at java.lang.NumberFormatException/forInputString (NumberFormatException.java:65).\nFor input string: \"articles-publish-idx\"\n",
 :clojure.main/triage
 {:clojure.error/class java.lang.NumberFormatException,
  :clojure.error/line 65,
  :clojure.error/cause "For input string: \"articles-publish-idx\"",
  :clojure.error/symbol java.lang.NumberFormatException/forInputString,
  :clojure.error/source "NumberFormatException.java",
  :clojure.error/phase :execution},
 :clojure.main/trace
 {:via
  [{:type java.lang.NumberFormatException,
    :message "For input string: \"articles-publish-idx\"",
    :at
    [java.lang.NumberFormatException
     forInputString
     "NumberFormatException.java"
     65]}],
  :trace
  [[java.lang.NumberFormatException
    forInputString
    "NumberFormatException.java"
    65]
   [java.lang.Long parseLong "Long.java" 692]
   [java.lang.Long parseLong "Long.java" 817]
   [clj_migratus$up$fn__3468 invoke "clj_migratus.clj" 53]
   [clojure.core$map$fn__5935 invoke "core.clj" 2772]
   [clojure.lang.LazySeq sval "LazySeq.java" 42]
   [clojure.lang.LazySeq seq "LazySeq.java" 51]
   [clojure.lang.Cons next "Cons.java" 39]
   [clojure.lang.RT boundedLength "RT.java" 1790]
   [clojure.lang.RestFn applyTo "RestFn.java" 130]
   [clojure.core$apply invokeStatic "core.clj" 669]
   [clojure.core$apply invoke "core.clj" 662]
   [clj_migratus$up invokeStatic "clj_migratus.clj" 54]
   [clj_migratus$up invoke "clj_migratus.clj" 51]
   [clj_migratus$_main invokeStatic "clj_migratus.clj" 77]
   [clj_migratus$_main doInvoke "clj_migratus.clj" 67]
   [clojure.lang.RestFn applyTo "RestFn.java" 137]
   [clojure.lang.Var applyTo "Var.java" 705]
   [clojure.core$apply invokeStatic "core.clj" 667]
   [clojure.main$main_opt invokeStatic "main.clj" 514]
   [clojure.main$main_opt invoke "main.clj" 510]
   [clojure.main$main invokeStatic "main.clj" 664]
   [clojure.main$main doInvoke "main.clj" 616]
   [clojure.lang.RestFn applyTo "RestFn.java" 137]
   [clojure.lang.Var applyTo "Var.java" 705]
   [clojure.main main "main.java" 40]],
  :cause "For input string: \"articles-publish-idx\""}}

Execution error (NumberFormatException) at java.lang.NumberFormatException/forInputString (NumberFormatException.java:65).
For input string: "articles-publish-idx"
yogthos commented 1 year ago

That makes sense to me, any chance you could do a PR for this?

It should be a pretty simple addition, the migrations namespace has find-migration-files and find-migration-resources functions, and it would just be a matter of factoring out the code that finds the files/resources and returns them as a list, then exposing it in the API.

ieugen commented 1 year ago

I also noticed that the commands to create a migration do not print out anything. I think it would be nice to provide some feedback to the user like "I created this file here" . I think it's similar for the other commands. This might be an issue in clj-migratus though.

ieugen commented 1 year ago

hi, would it be ok to add a migratus.cli namespace and use clojure.tools.cli to wrap migratus core ?! This will add a dependency - but only if you use the ns. The logic should be straight forward I think ?!

From @yogthos , on Slack:

oh yeah migrations.cli sounds good, clojure.tools.cli is a pretty small dependency, so that's not too much of a concern 14:38 I've actually not used clj-migratus much as I tend to do migrations from the app and use the REPL during dev

yogthos commented 1 year ago

Oh yeah, adding a message for creating the files sounds like a good idea as well.

sandre1 commented 1 year ago

Hi, i would like to work on this issue. Here are some ideas on cli design:

migratus --config CONFIG init
migratus create NAME
migratus migrate
migratus migrate --until-just-before ID
migratus reset
migratus rollback
migratus rollback --until-just-after ID
migratus up IDS
migratus down IDS
migratus list 

list

filter

format

MIGRATION-ID DESCRIPTION APPLYED PATH 20231002193400 create-user 2023-10-02 16:06:33.594 path/to/file 20231102193400 articles 2023-11-02 16:06:33.594 20231202193400 create-user
20231204193400 articles


What do you guys think?
yogthos commented 1 year ago

That sounds reasonable, I think maybe first step could be to handle this at the API level, and then add a CLI namespace that would allow running the commands from the command line. I think for the API part, I'd like to just keep adding flags to the existing config map.

sandre1 commented 1 year ago

Hello, i have made a PR https://github.com/yogthos/migratus/pull/244 with some work in progress. If you have the time to give me some input, it will help me a lot. Thanks.

sandre1 commented 1 year ago

Hi,

I have implemented a CLI option to set the log level with --verbose: https://github.com/yogthos/migratus/pull/245

As @ieugen said:

I can definitely see value in a migrations library being able to produce a list of all possible migrations, indicating which have been applied an which haven't (if the system is able to track on a per-migration basis whether it has been applied or not). Our hand-rolled system at work is "linear" (it has a monotonically-increased "version" number and just tracks the highest version applied) and if I was starting over again, I'd use Migratus and I would love the "report" feature that provided more information, especially if it could list the date a migration was applied, etc.

It could be valuable to be able to show in CLI more info when using migratus:

I propose to create another function, similar to migratus.database/completed-ids -> completed which will expose to the API the applied date and descripton of a migration

What do you think? Thank you

ieugen commented 1 year ago

I think a PR to add completed API is ok IMO. WDYT @yogthos ?

yogthos commented 1 year ago

yeah that sounds good to me 👍

sandre1 commented 11 months ago

https://github.com/yogthos/migratus/pull/251 Hello, i have added API support to display more info about migrations in CLI:

It is a draft at the moment;

Thanks.