Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to list ALL migrations considered by migratus #240

Open
ieugen opened this issue Jun 14, 2023 · 13 comments
Open

Add option to list ALL migrations considered by migratus #240

ieugen opened this issue Jun 14, 2023 · 13 comments
Assignees

Comments

@ieugen
Copy link
Collaborator

ieugen commented Jun 14, 2023

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 ieugen self-assigned this Jun 14, 2023
@ieugen ieugen changed the title Add option to list ALL migrations Add option to list ALL migrations considered by migratus Jun 14, 2023
@ieugen
Copy link
Collaborator Author

ieugen commented Jun 14, 2023

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
Copy link
Collaborator Author

ieugen commented Jun 14, 2023

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
Copy link
Owner

yogthos commented Jun 14, 2023

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
Copy link
Collaborator Author

ieugen commented Jun 15, 2023

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
Copy link
Collaborator Author

ieugen commented Jun 15, 2023

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
Copy link
Owner

yogthos commented Jun 15, 2023

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

@sandre1
Copy link
Contributor

sandre1 commented Jun 21, 2023

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

  • available migrations - "--available" default option, applyed and non applyed, discovered by migratus
  • applyed migrations - "--applyed", from db
  • pending migrations - "--pending", non applyed

filter

  • other variant to list migrations using a filter
    --filter "available", default option, other options "applyed", "pending"
migratus list --filter=available

format

  • --format "plain", default option, other options "json", "edn"

  • output example

migratus list

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
Copy link
Owner

yogthos commented Jun 21, 2023

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 sandre1 mentioned this issue Jul 12, 2023
5 tasks
@sandre1
Copy link
Contributor

sandre1 commented Jul 12, 2023

Hello, i have made a PR #244 with some work in progress.
If you have the time to give me some input, it will help me a lot.
Thanks.

@sandre1 sandre1 mentioned this issue Jul 25, 2023
3 tasks
@sandre1
Copy link
Contributor

sandre1 commented Jul 25, 2023

Hi,

I have implemented a CLI option to set the log level with --verbose:
#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:

  • date am migration was applied ("completed")
  • description text of the migration?!

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
Copy link
Collaborator Author

ieugen commented Jul 25, 2023

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

@yogthos
Copy link
Owner

yogthos commented Jul 25, 2023

yeah that sounds good to me 👍

@sandre1
Copy link
Contributor

sandre1 commented Aug 22, 2023

#251
Hello,
i have added API support to display more info about migrations in CLI:

  • list migrations by type, with id, name and completion date
  • list migrations in .edn and .json with cli-option --format FILE-FORMAT

It is a draft at the moment;

Thanks.

ieugen added a commit that referenced this issue Oct 31, 2023
* BREAKING? Enhanced Store protocol with close alias for disconnect
* BREAKING? connect now returns the store (this)
* create (migration) fn now resolves absolute file path for migration
* Enhanced docs for Store protocol
* CLI can load config from file
* CLI can load config from env
* CLI can load config from cli args
* CLI merges configs in this order: file, env, args
* Added some tests for CLI parsing
* Added status command, to display migration status:
  - connection info - so we know which server we are connecting to
  - migrations directory - so we know where we get migrations from
  - the latest applied migration
  - the list of not-applied migrations
  - any migrations applied and not present ?!
ieugen added a commit that referenced this issue Oct 31, 2023
* BREAKING? Enhanced Store protocol with close alias for disconnect
* BREAKING? connect now returns the store (this)
* create (migration) fn now resolves absolute file path for migration
* Enhanced docs for Store protocol
* CLI can load config from file
* CLI can load config from env
* CLI can load config from cli args
* CLI merges configs in this order: file, env, args
* Added some tests for CLI parsing
* Added status command, to display migration status:
  - connection info - so we know which server we are connecting to
  - migrations directory - so we know where we get migrations from
  - the latest applied migration
  - the list of not-applied migrations
  - any migrations applied and not present ?!
ieugen added a commit that referenced this issue Oct 31, 2023
* BREAKING? Enhanced Store protocol with close alias for disconnect
* BREAKING? connect now returns the store (this)
* create (migration) fn now resolves absolute file path for migration
* Enhanced docs for Store protocol
* CLI can load config from file
* CLI can load config from env
* CLI can load config from cli args
* CLI merges configs in this order: file, env, args
* Added some tests for CLI parsing
* Added status command, to display migration status:
  - connection info - so we know which server we are connecting to
  - migrations directory - so we know where we get migrations from
  - the latest applied migration
  - the list of not-applied migrations
  - any migrations applied and not present ?!
ieugen added a commit that referenced this issue Nov 4, 2023
ieugen added a commit that referenced this issue Nov 6, 2023
* Implemnted status command that works mostly like list
ieugen added a commit that referenced this issue May 20, 2024
* BREAKING? Enhanced Store protocol with close alias for disconnect
* BREAKING? connect now returns the store (this)
* create (migration) fn now resolves absolute file path for migration
* Enhanced docs for Store protocol
* CLI can load config from file
* CLI can load config from env
* CLI can load config from cli args
* CLI merges configs in this order: file, env, args
* Added some tests for CLI parsing
* Added status command, to display migration status:
  - connection info - so we know which server we are connecting to
  - migrations directory - so we know where we get migrations from
  - the latest applied migration
  - the list of not-applied migrations
  - any migrations applied and not present ?!
ieugen added a commit that referenced this issue May 20, 2024
ieugen added a commit that referenced this issue May 20, 2024
* Implemnted status command that works mostly like list
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants