Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork119
fix: tenant migrations should honor module attributes#643
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
71577c9 to3073ca0Comparesuch as @disable_ddl_transaction and @disable_migration_lock
3073ca0 to89c913bCompareWhile this fix may work--I'm not 100% certain given the difficulty in testing. And there's another complication. You still can't do do schema migrations that use the Maybe the migration generator needs to do something like this createindex(:posts,[:title],concurrently:Mix.env()!=:test) |
Yeah, that makes sense 🤔 We probably will need to figure some other kind of thing out here because you could also be running this in an action that is in a transaction. I'm not sure what the right answer is? Maybe some kind of error if you are in a transaction currently, that can be silenced in test? |
Yeah, it will error in a transaction so at least you’ll know it failed. What about changing the generated migration to include some helpers?For example you only need it to be concurrent if there are existing records. So a new tenant doesn’t need to concurrency on for the initial migrations. …On Oct 20, 2025 at 6:13 PM -0700, Zach Daniel ***@***.***>, wrote: zachdaniel left a comment (ash-project/ash_postgres#643) Yeah, that makes sense 🤔 We probably will need to figure some other kind of thing out here because you could also be running this in an action that is in a transaction. I'm not sure what the right answer is? Maybe some kind of error if you are in a transaction currently, that can be silenced in test? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you modified the open/close state.Message ID: ***@***.***> |
That is a good point. Then future migrations are applied "out of band" out of a transaction. Smart 👍 |
Uh oh!
There was an error while loading.Please reload this page.
such as @disable_ddl_transaction and @disable_migration_lock
Fixes#610
I was not able to get a failing test for this bug. Ecto would not error, but would give a warning. I have two tests and one generates a warning and the other does not. Since they both pass (even with warnings), it's not a reliable regression test.
Contributor checklist
Leave anything that you believe does not apply unchecked.