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

Avoid accidental replacement of main DB model connections #98

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kaorukobo
Copy link

@kaorukobo kaorukobo commented Feb 20, 2024

fixes #97

Notes on CI failures

Tests for Rails 7.1 do not pass even at commit aafa5b2

@kaorukobo
Copy link
Author

kaorukobo commented Feb 26, 2024

After making this PR, I ran into another issue in multiple database usage, which may be related to #24 and #47.
I'll put more details in #97, the issue bound to this PR. [EDITED on Mar 3: instead I posted another suggestion to #100, where this PR can be closed if it is approved.]

In short, this patch will:

  • resolves the issue with the use of a :truncation strategy.
  • but be useless with the use of :transaction strategy due to the "another issue" above.

Copy link
Member

@etagwerker etagwerker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaorukobo Is this something that could be fixed by #108 instead?

@kaorukobo
Copy link
Author

kaorukobo commented Jul 12, 2024

@etagwerker I checked if #97 is reproduced with the commit of the PR. The problem still remains.

As per I wrote above and commented here, I would rather suggest dropping support for the connection name in the :db option.

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

Successfully merging this pull request may close these issues.

Accidental replacement of main DB model connections in multi-database env
2 participants