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 case when we want a UrlConfig but the URL is nil #35102

Merged

Conversation

eileencodes
Copy link
Member

Previously if the url key in a config hash was nil we'd ignore the
configuration as invalid. This can happen when you're relying on a
DATABASE_URL in the env and that is not set in the environment.

production:
  <<: *default
  url: ENV['DATABASE_URL']

This PR fixes that case by checking if there is a url key in the
config instead of checking if the url is not nil in the config.

In addition to changing the conditional we then need to build a url hash
to merge with the original hash in the UrlConfig object.

Fixes #35091

cc/ @gmcgibbon @msdundar this should fix the issue you opened.

Previously if the `url` key in a config hash was nil we'd ignore the
configuration as invalid. This can happen when you're relying on a
`DATABASE_URL` in the env and that is not set in the environment.

```
production:
  <<: *default
  url: ENV['DATABASE_URL']
```

This PR fixes that case by checking if there is a `url` key in the
config instead of checking if the `url` is not nil in the config.

In addition to changing the conditional we then need to build a url hash
to merge with the original hash in the `UrlConfig` object.

Fixes rails#35091
@eileencodes eileencodes added this to the 6.0.0 milestone Jan 30, 2019
@eileencodes eileencodes self-assigned this Jan 30, 2019
@eileencodes eileencodes merged commit 677b658 into rails:master Jan 30, 2019
@eileencodes eileencodes deleted the fix-case-when-url-in-url-config-is-nil branch January 30, 2019 15:26
@msdundar
Copy link

@eileencodes I confirm the fix, it's now working as it was before.

Thanks for this PR and for all your amazing contributions to the Rails 🎉

@eileencodes
Copy link
Member Author

Thanks for following up @msdundar! And thank you for testing the Rails 6 beta so we could find and fix this issue before the final release ❤️

msdundar added a commit to omu/nokul that referenced this pull request Jan 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rails 6.0.0.beta1 can not load database configuration when the URL (or database) is nil
2 participants