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 back username and port as AR configuration values #188

Closed
wants to merge 1 commit into from

Conversation

tjchambers
Copy link

In Rail 6.1 the information return is from an ActiveRecord::DatabaseConfigurations::HashConfig object, with methods for some but not all of the configuration attributes. This fixes a bug wheree database and host had getter methods, but the username and port values did not. All values are now just fetched from the instance variable configuration_hash directly.

This fixes #187

In Rail 6.1 the information return is from an `ActiveRecord::DatabaseConfigurations::HashConfig` object, with methods for some but not all of the configuration attributes. This fixes a bug wheree `database` and `host` had getter methods, but the `username` and `port` values did not. All values are now just fetched from the instance variable configuration_hash directly.

Signed-off-by: Tim Chambers <tim@possibilogy.com>
@SamSaffron
Copy link
Member

I worry a bit about cross compatibility here. What is going to happen with earlier versions of rails?

@tjchambers
Copy link
Author

tjchambers commented Sep 23, 2021

You separated the pre 6.1 code out, which is all that I changed. I mean I only changed post 6.0 code. And in our project we are running both 6.0 and 6.1 and now they run the same.

@SamSaffron
Copy link
Member

Happy to try this, any idea why CI is failing?

@tjchambers tjchambers closed this Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

ALLOWED Config Labels is inaccurate for Rails 6.1
2 participants