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

Add an option database_type #338

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

Conversation

markusherzog
Copy link

Problem: When using numeric_ordering: true, closure_tree tries to find out the type of database during the initialization process. This can be problematic, for example when doing rails db:create, in which case there is no database, yet. Another scenario is rails asssets:prcompile in a Docker container that is being made for production, but it doesn't have access to the production database yet, because it isn't rolled out, yet.

Solution: I have added an option to specify the database_type manually. This option is only used then numeric_ordering is set to true

This relates to issue #264 and PR #306

@kbrock
Copy link
Contributor

kbrock commented Sep 10, 2019

This is nice.
Detecting the database type in the code is a little problematic because it often leads to connecting to the database prematurely. Especially while loading the ruby classes into memory.

Alternatively, there is a case statement that needs to run every method call at runtime.

This seems like a reasonable compromise between auto configuration and optimization

@n-rodriguez
Copy link
Contributor

@markusherzog can you please rebase your branch on master?

@seuros I think it's the easiest way to handle Rails 6.1 multi DB. wdyt?

Problem: When using `numeric_ordering: true`, closure_tree tries to find out the type of database during the initialization process. This can be problematic, for example when doing `rails db:create`, in which case there is no database, yet. Another scenario is `rails asssets:prcompile` in a Docker container that is being made for production, but it doesn't have access to the production database yet, because it isn't rolled out, yet.

Solution: I have added an option to specify the database_type manually. This option is *only* used then numeric_ordering is set to `true`
@markusherzog
Copy link
Author

@n-rodriguez sure, i've rebased my branch now (sorry for the delay ;-))

@seuros
Copy link
Member

seuros commented Jan 19, 2024

This will be figured out in runtime.

We finally dropped the support to version that caused error.

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.

4 participants