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

After Re-open REPL, migration exception will throw when go #1

Open
zerg000000 opened this issue Jun 27, 2017 · 11 comments
Open

After Re-open REPL, migration exception will throw when go #1

zerg000000 opened this issue Jun 27, 2017 · 11 comments

Comments

@zerg000000
Copy link

Currently, if we changed the migration string, (dev) and (go), then ragtime will simply throw

IllegalArgumentException No implementation of method: :run-down! of protocol: #'ragtime.protocols/Migration found for class: nil clojure.core/-cache-protocol-fn (core_deftype.clj:583)

If we already (dev) and (go), then change the migration string and (reset) we have no exception.

it seems due to lack of index which have the previous auto-hash-ids

(defmethod ig/init-key :duct.migrator/ragtime [_ options]
  (migrate {} options))

(defmethod ig/resume-key :duct.migrator/ragtime [_ options _ index]
  (migrate index options))

It would be better to warn instead of throwing unknown exception.

@zerg000000
Copy link
Author

when DB File System REPL index effect
first go abcd#1 migrate abcd#1
abcd#1 abcd#1 abcd#1
change abcd abcd#1 abcd#2 abcd#1
comment/uncomment and reset abcd#1 abcd#2 abcd#1 rollback abcd#1, migrate abcd#2
abcd#2 abcd#2 abcd#2
reopen repl, with unapplied changes abcd#2 abcd#3
go abcd#2 abcd#3 try to rollback abcd#2 and throw exception, since abcd#2 not in index

@weavejester
Copy link
Contributor

weavejester commented Jun 27, 2017

Yes, a better error message would be useful here. The Ragtime migrator can't rollback migrations it doesn't have in memory.

One thing I'm considering is to make Ragtime's migrations serializable via something like Nippy, and then add them to a blob in the ragtime_migrations table.

@zerg000000
Copy link
Author

it depends on the designer decision.

but "serialization" solution might be quite hard. what if the serialized .down.sql is not valid? During the development time, .up.sql may be okay, since it must be run before serializing to ragtime_migrations, however, .down.sql would be never tested before serializing to ragtime_migrations. This might cause the rollback blocked forever.

@weavejester
Copy link
Contributor

weavejester commented Jun 28, 2017

Yes, that's true, though I want to encourage higher-level migrations in future so that people aren't writing up and down migrations manually. That said, I expect there will always be some migrations that we'll need to write in bare SQL.

Another option might be to simply to clear the database if a migration cannot be rolled back (obviously in development only).

@isker
Copy link

isker commented Jul 8, 2018

Another option might be to simply to clear the database if a migration cannot be rolled back (obviously in development only).

Having this sort of nuclear option in some form sounds attractive to me as I've had to do it manually to the database in development after having written bad migrations.

@alekseysotnikov
Copy link
Contributor

alekseysotnikov commented Jun 25, 2021

@isker In this case, I am simply drop a DB schema (or schemas) and create again before evaluating (go)

-- Postgres
DROP SCHEMA public CASCADE;
CREATE SCHEMA public;

@hden
Copy link

hden commented Nov 18, 2021

So what's the current solution if we want to edit the migration string or file, then let ragtime rollback the migration and re-apply the now-changed migration?

(for local development obviously)

@weavejester
Copy link
Contributor

It just needs the applied migration to be in memory. So currently you migrate, then work on the migration, then migrate again, etc.

@hden
Copy link

hden commented Nov 18, 2021

Got it. Thanks.

@zerg000000
Copy link
Author

zerg000000 commented Dec 2, 2024

After using it for a long time, I have some suggestions to make it easier for certain usage patterns.

  1. A force execution of the last .down.sql ignoring the hash is useful for recovering from the problem mentioned in this issue.
  2. By default, the migrations config should go into a separate file. the config.edn is not very readable since it grows long in a normal long-running commercial project.
  3. Remove usage of #duct/resource and #ig/ref when defining the migration. Extending the original ragtime way of loading these files should be the way to go since the coupling doesn't bring extra functionality.
  4. In addition to 2, providing a separate (and lightweight) way to run the migrations for deployment is a good addition, I eventually created a lambda for migrating, to prevent the migration from being run multiple times during the deployment.
  5. A cli command for generating the .up.sql, .down.sql files and updating migrations.edn file would be a very nice addition. ( I also added this to my project. )

Edit:

The current changes already fix 2,3 points! marvelous!

@weavejester
Copy link
Contributor

Thanks for the feedback!

I've actually already addressed points 2 and 3 in the latest version, which loads migrations from an edn file. I've also added syntax sugar for common operations that should reduce mistakes.

A migration file might now look like:

[[:create-table example
  (id "INTEGER PRIMARY KEY")
  (name "TEXT")
  (modified_at "TIMESTAMP DEFAULT CURRENT_TIMESTAMP")]

 [:create-unique-index example_name (name)]

 {:id "custom_migration"
  :up ["..."]
  :down ["..."]}]

I'm currently working on making the migration index persistent across REPL restarts.

You can migrate a Duct application without starting it just by limiting the keys to :duct/migrator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants