weavejester / ragtime

Database-independent migration library
Eclipse Public License 1.0
610 stars 85 forks source link

Add helper for creating new migrations to ragtime.repl #122

Closed jivagoalves closed 6 years ago

jivagoalves commented 6 years ago

Hi @weavejester , first of all thanks for creating this library!

In our app, we have built a helper to create migrations based on an EDN template using the current timestamp as part of the filename to guarantee a timeline of migrations. This way we standardize how migrations get created for the app. Creating migrations is just a matter of entering a description and the file will be created following a convention and a migration template. At the end, you just have to type your actual migration in the file. If you are interested in adding this feature to your library, I think we can contribute. Below is an example of usage:

user=> (add-migration "create-my-table")
Created resources/migrations/20180113131519-create-my-table.edn

user=> (println (slurp "resources/migrations/20180113131519-create-my-table.edn"))
{:up   ["CREATE TABLE foo (id int);"]
 :down ["DROP TABLE foo;"]}

What do you think?

weavejester commented 6 years ago

I think I'd like to make any templates part of a separate library, as what should be in the templates is rather a matter of opinion. For example, should add-migration create:

{:up   ["CREATE TABLE foo (id int);"]
 :down ["DROP TABLE foo;"]}

Or:

{:up   [""]
 :down [""]}

Or should it generate an "up" and "down" SQL file? Or any number of other possibilities.

jivagoalves commented 6 years ago

Yes, absolutely. My idea was to have the users to provide a :migration-template as part of the config:

(ns my.app)

(def migration-template
  "{:up   [\"CREATE TABLE foo (id int);\"]\n :down [\"DROP TABLE foo;\"]}")

(defn add-migration
  [desc]
  (let [ragtime-config {:datastore  (ragtime.jdbc/sql-database db-spec)
                        :migrations (ragtime.jdbc/load-resources "migrations")
                        :migration-template migration-template}]
    (ragtime.repl/add-migration ragtime-config desc)))

I'd definitely leave it for the users of the library to decide them, so ragtime would just glue things together and auto generate, for instance, the sequence id and create the actual file based from the template. My example is for edn but the same idea would apply for sql too. Perhaps two methods like ragtime.repl/add-edn-migration and ragtime.repl/add-sql-migration.

danielcompton commented 6 years ago

What do we gain by adding integration points into ragtime? What does that buy us over creating the migrations by a wholly separate library without any knowledge of ragtime? This might be a failure of imagination on my part :)

jivagoalves commented 6 years ago

Hey @danielcompton , thanks for your input!

I didn't get what you mean by integration points. I don't see any here other than the file system. I'm not concerned with ragtime taking care of creating migration files itself since it is already concerned with creating the migration table and doing the bookkeeping over there. I don't have a good point against a separate library other than keeping migration concerns in a single place (this library) and having just one dependency to worry about in my project. In other words, no issues with lib A does not work with version of lib B, etc. I simply don't see requirements enough to separate them but please let me know if I'm missing something.

danielcompton commented 6 years ago

I simply don't see requirements enough to separate them but please let me know if I'm missing something.

I'll defer to @weavejester but I usually start from the opposite perspective of "What requires these to be connected"? It sounds like a useful tool (one that I've wanted too), but I'm just not sure if it should be part of Ragtime? I'm not strongly opposed, just raising the question.

weavejester commented 6 years ago

I agree with @danielcompton. There are a lot of different ways to create file templates, and rather than adding a templating system to Ragtime, I'd prefer to have it as a separate library.

jivagoalves commented 6 years ago

I'll close this issue then. Please let me know if you guys have something already in mind for this separate library. I'd like to contribute if possible. Thanks!