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

(migratus/initialize config) #85

Open
MafcoCinco opened this issue Jul 26, 2016 · 32 comments
Open

(migratus/initialize config) #85

MafcoCinco opened this issue Jul 26, 2016 · 32 comments

Comments

@MafcoCinco
Copy link

First, Migratus has been great. Tight and simple migration tool.

Having said that, I would like the ability to create the database itself. Currently, AFAIK, the user must initialize the database (schema) outside the scope of the project (i.e. CREATE SCHEMA <schema_name>). Other migration tools that I have used (specifically in the context of Ruby on Rails) provide a setup or initialize function which allows the user to generate the schema(s) from a clean DB. Would be awesome to have in Migratus as it would provide a uniform interface for initializing the DB and make the project more self contained.

If I'm missing something and this is already possible, I apologize.

@yogthos
Copy link
Owner

yogthos commented Jul 26, 2016

I think that would be a reasonable idea. It sounds like it would just be a matter of having an init function that would check if the db exists, and run a SQL script if none is found. So, in addition to the standard migration files there would be something like init.sql for the schema creation?

@MafcoCinco
Copy link
Author

Yep, exactly.

@yogthos
Copy link
Owner

yogthos commented Jul 26, 2016

Yeah I think that would be easy enough to add. I'll take a look when I get a chance, but if it's something you think you'd like to try to add I'm open to a pr as well.

@yogthos
Copy link
Owner

yogthos commented Jul 28, 2016

I just checked in an update that adds an init function for running the initialization script. It looks for the init.sql by default, and a custom name can be supplied using the :init-script key in the config. If you can try it out locally and let me know that it works as expected, I can then push out a new release with the feature.

@MafcoCinco
Copy link
Author

I'm getting an error and I think it has to do with the connection string. When the DB/schema is present, connection string is something like mysql://host/schema. Since there isn't a schema when init is run, should I create a separate connection configuration for use with init, dropping the schema portion of the URI?

@yogthos
Copy link
Owner

yogthos commented Jul 28, 2016

Yeah, looks like you would need to use a different connection for that.

@MafcoCinco
Copy link
Author

Yep. Makes sense. Need to connect without selecting a DB/Schema. Might want to add that to the doc string and/or regular documentation.

@yogthos
Copy link
Owner

yogthos commented Jul 28, 2016

Yup, I'll update the docs once I push out the new release. So, once you use a different connection string everything is working as expected then?

@MafcoCinco
Copy link
Author

MafcoCinco commented Jul 28, 2016

Actually, no. When I run the normal migration, I get an error I would expect (Unknown database) as the DB schema has not yet been created. However, running with the same config minus the schema in the connection string, I get something totally different: java.lang.IllegalArgumentException: db-spec null is missing a required parameter. It makes me think that the configuration object is not getting passed along correctly within Migratus for the init function? Or perhaps there is an option in the DB connection config that I need to add that is not required for regular migrations?

Not 100% sure but things look correct in my client code.

@yogthos
Copy link
Owner

yogthos commented Jul 28, 2016

Currently, you have to call connect explicitly to initialize the connection:

(let [store (proto/make-store config)]
      (proto/connect store)
      (proto/init store)
      (proto/disconnect store))

Perhaps init should try to connect automatically though.

@MafcoCinco
Copy link
Author

I see. I was calling (migratus/init config) just like (migratus/migrate config) or (migratus/rollback). Looks like there is a little bit more to it currently but I agree, it should probably connect automatically like the other functions do. Let me try your code.

@MafcoCinco
Copy link
Author

different error but still an error :-(. Getting clojure.lang.PersistentVector cannot be cast to clojure.lang.Named when calling proto/connect. Is there any additional munging that needs to happen on the store object from what would normally be passed to migratus/migrate?

@yogthos
Copy link
Owner

yogthos commented Jul 28, 2016

Hmm, take a look here at how the test is setup. The config in the test looks as follows:

(def config {:store                :database
             :migration-dir        "migrations/"
             :migration-table-name "foo_bar"
             :db                   {:classname   "org.h2.Driver"
                                    :subprotocol "h2"
                                    :subname     db-store}})

@MafcoCinco
Copy link
Author

Configuration options look correct. Some values are different. I'm using mysql instead of h2. My classname is com.mysql.jdbc.Driver (using version 0.3.7 of org.clojure/java.jdbc). It looks similar to this issue (at least the error it is reporting is the same).

@yogthos
Copy link
Owner

yogthos commented Jul 29, 2016

Yeah, clojure.java.jdbc had breaking changes in the API, and connect tries to create a migration table if it doesn't exist using the new API.

@MafcoCinco
Copy link
Author

Yeah. Saw that in the log file in the line before the error. Is it possible to turn off migration table creation for the init phase of the migration?

@yogthos
Copy link
Owner

yogthos commented Jul 29, 2016

You could just create the migration table manually. That step will be skipped when it exists.

@MafcoCinco
Copy link
Author

I don't think I can exactly. The migration table lives in the schema that I'm trying to create with the init file. I could create it in the init file but that is less than ideal and, more importantly, I'm not sure it will work. Is it testing for the existence of the migration table prior to running the init file? Even if it works, probably don't want to force a user to create a book keeping table that is an implementation detail to the library.

@yogthos
Copy link
Owner

yogthos commented Jul 29, 2016

That's a good point actually, the init can't rely on the connect function since that one assumes a schema. I'll update to have it do its own connection.

@yogthos
Copy link
Owner

yogthos commented Jul 29, 2016

ok just pushed a fix for that

@MafcoCinco
Copy link
Author

The schema creation was successful, so that is good. However, when I tried to follow that with a run of my migrations, they failed with the same error the init was getting before (PersistentVector casting issue).

@yogthos
Copy link
Owner

yogthos commented Jul 29, 2016

Right, but that's a problem with the library version. The workaround in this case would be to create the migration table in the schema creation script.

@MafcoCinco
Copy link
Author

Okay. I can do that. When you say library version, you mean the JDBC version I'm using?

@MafcoCinco
Copy link
Author

MafcoCinco commented Jul 29, 2016

Just tried to create schema_migrations table via init.sql with no luck. Here is the contents of init.sql, which I think is correct syntax for running multiple statements in a single file:

CREATE SCHEMA database;
--;;
CREATE TABLE database.schema_migrations;

Gives a very unhelpful (love MySQL error reporting!) SQL syntax error

@yogthos
Copy link
Owner

yogthos commented Jul 29, 2016

Yeah that part I'm not sure about. I'm not too familiar with MySQL.

@hyleung
Copy link

hyleung commented Aug 25, 2016

@MafcoCinco I was trying to do the same thing with MySQL. Oddly enough, when I put the CREATE DATABASE command in the init.sql script and use a separate migration script for creating the table, it seems to work.

The multi-command syntax works fine when I do it that way, but errors when I have it in the init.sql script. ¯_(ツ)_/¯

@MafcoCinco
Copy link
Author

@hyleung Thanks for the update. I will give that a try.

@hyleung
Copy link

hyleung commented Aug 25, 2016

@MafcoCinco btw, I should add - you might want to use a different config to create the schema vs. execute the migrations, cos there'll be a table created to track schema_migrations in whatever schema you connect to initially. For example, I connected using localhost/mysql and the tracking table was created there, which might not be desirable.

@J-ogawa
Copy link

J-ogawa commented May 8, 2017

@hyleung I met similar error at in [migratus "0.9.3"] about MySQL. But no error in [migratus "0.8.13"].

@dijonkitchen
Copy link
Contributor

Perhaps something like templating the scripts like here: https://github.com/rails/rails/blob/master/activerecord/Rakefile#L179 ?

@lane-s
Copy link

lane-s commented Jun 10, 2019

I'm having trouble with the difference in connections as well. When running init.sql I need to use the subname //localhost:5432/postgres, but then my migrations will either be using //localhost:5432/my_db or //localhost:5432/test_db. Would it be possible to add a :init-subname key to the config?

@yogthos
Copy link
Owner

yogthos commented Jun 10, 2019

Yeah that sounds reasonable to me. Might be better to just have a separate optional :init-db key, and when it's present the connection config will be read from there rather than the one defined in :db. I'm not sure I'll have time to look at this in the near future, but I can help out with a PR. It should be a fairly small change.

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

6 participants