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

Fix multiple database support for DATABASE_URL env variable #36756

Merged

Conversation

@seejohnrun
Copy link
Contributor

commented Jul 24, 2019

Summary

This PR fixes an issue where multi-database configurations were incompatible with setting a DATABASE_URL environment variable. We had been using config["database"] || config["adapter"] || ENV["DATABASE_URL"] to determine if a multi-tier config was in place, which made the assumption that if DATABASE_URL was present, we were in a single-database configuration.

As part of this work, this PR also includes a light refactor to make both multi and single database configurations lead into the same code path so they behave the same.

Other information

As mentioned in #36736, this regression was introduced as part of f2ad69f

Thank you for the great bug report and reproduction!


Closes #36736

/cc @morgoth @eileencodes

Fix multiple database support for DATABASE_URL env variable
This commit fixes an issue where multi-database configurations were
incompatible with setting a `DATABASE_URL` environment variable.

As part of this work, this commit also includes a light refactor
to make both multi and single database configurations lead into the same
code path so they behave the same.

As mentioned in #36736, this regression was introduced as part of
f2ad69f

@rails-bot rails-bot bot added the activerecord label Jul 24, 2019

@eileencodes eileencodes merged commit 49b531b into rails:master Jul 24, 2019

2 checks passed

buildkite/rails Build #62453 passed (9 minutes, 38 seconds)
Details
codeclimate All good!
Details

eileencodes added a commit that referenced this pull request Jul 24, 2019

Merge pull request #36756 from seejohnrun/env-urls-with-multiple-data…
…bases

Fix multiple database support for DATABASE_URL env variable
@eileencodes

This comment has been minimized.

Copy link
Member

commented Jul 24, 2019

Thanks @seejohnrun!

Backported this to 6-0-stable 😄

walk_configs(env_name.to_s, "primary", config)
end.flatten.compact
db_configs = configs.flat_map do |env_name, config|
if config.is_a?(Hash) && config.all? { |k, v| v.is_a?(Hash) }

This comment has been minimized.

Copy link
@eileencodes

eileencodes Jul 24, 2019

Member

Minor but tomorrow when we pair on this let's make sure to underscore the k since it's not used.

@eileencodes eileencodes added this to the 6.0.0 milestone Jul 25, 2019

@seejohnrun seejohnrun deleted the seejohnrun:env-urls-with-multiple-databases branch Jul 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.