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

[Idea] Ignore migrations from non-supported adapters #255

Closed
jenskdsgn opened this issue Feb 27, 2024 · 5 comments
Closed

[Idea] Ignore migrations from non-supported adapters #255

jenskdsgn opened this issue Feb 27, 2024 · 5 comments

Comments

@jenskdsgn
Copy link
Contributor

Hey,

first of all thanks for this helpful gem! In my current situation I am trying to add a second database as part of Rails' multi database configuration. The primary db will be postgres and secondary clickhouse, both databases will be owned by the rails app including migrations. The current behaviour from looking at strong_migration's code, it would pick the AbstractAdapter and would raise an error on setting the timeout here.

So the two options I am seeing here is

  1. Create a clickhouse adapter and add it to your code base
  2. Ignore migrations from databases with unsupported adapters

Since the support of multiple databases on Rails, it would be nice to have an option to pick on which databases to apply this gem.

@ankane
Copy link
Owner

ankane commented Feb 28, 2024

Hi @jenskdsgn, thanks for the suggestion! Added a warning for unsupported adapters and an option to ignore databases in the ignore branch, but want to figure out a better name (or pattern) before merging.

@jenskdsgn
Copy link
Contributor Author

Wow. Thanks for the prompt reply. I'll test this and let you know if it works!

@jenskdsgn
Copy link
Contributor Author

Ok, I had the time to check it. Unfortunately when ignoring the database, the checks are ignored, which is good, however the migration didn't execute either.

I think the offending line is here:

https://github.com/ankane/strong_migrations/compare/ignore#diff-70c919b24b6428e59eef896cbbd7f567bde86e86d5649af3dc6330fbe9c974c4R31

I believe it should be:

return yield if !enabled?

if I understand it correct, the checker class executes the migration in the block after the check. If you guard the perform with enabled it won't execute the block, which is not the expected behavior. I want to disable the strong migration check and not the whole migration.

@lucas-jt
Copy link

Hello @ankane 👋

Would it be possible to merge the changes from the ignore branch into master? This is an optional configuration, so it wouldn't change the gem's behavior.

I need this functionality because we are also using ClickHouse. As the ignore branch is outdated, we plan to create and maintain an internal fork.

Is there any specific reason for not adding this to master?

@ankane ankane closed this as completed in 6efff12 Nov 1, 2024
@ankane
Copy link
Owner

ankane commented Nov 1, 2024

Added a skip_databases option in the commit above, which will be included in 2.1. Thanks for the suggestion and PR fix @jenskdsgn!

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

3 participants