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 support for 3-tier database configurations on development #182

Conversation

horacio
Copy link

@horacio horacio commented Jun 9, 2020

Hi! I noticed parity doesn't get along with multi-database configurations, so I was thinking it could be nice to support this. This patch shows a rough idea of the needed changes, but I wanted to make sure I'm heading in the right direction. Any comments would be greatly appreciated.

@horacio horacio closed this Sep 28, 2020
@geoffharcourt
Copy link
Collaborator

Hi @horacio I am quite sorry for missing this PR, it came up while I was very short on time. I think this looks like it's workable. If you re-open it I can approve this week.

@geoffharcourt geoffharcourt reopened this Sep 28, 2020
@horacio
Copy link
Author

horacio commented Sep 28, 2020

@geoffharcourt don't worry, and thanks for re-opening it! I'd be glad to work on this again. I'll continue with the specs for this addition, if that's OK.

@geoffharcourt
Copy link
Collaborator

@horacio sounds great. Tag me when you're ready with specs and I'll review again. Thanks for your time and effort.

@horacio horacio force-pushed the hb-add-multi-database-support-on-development branch from 6b355f2 to bb9c454 Compare October 7, 2021 23:51
@horacio
Copy link
Author

horacio commented Oct 8, 2021

Hey @geoffharcourt, first of all I just wanted to apologize for taking more than a year to get to this.

I both rebased against master and added a few changes to the PR, and I was wondering about your thoughts on supporting this and what do you think would be needed to accomplish that. Thanks!

@horacio
Copy link
Author

horacio commented Oct 10, 2021

@croaky Hey there! Could you please, whenever you can, take a look at this changes? Do you think they make sense? What would be needed to move this forward? Thanks in advance.

@geoffharcourt
Copy link
Collaborator

@horacio sorry for the lack of feedback here. Can you squash your 5 commits down to 1 commit, make sure the commit message and body explain the problem and why we're making this change, and then I can merge it and cut a release this week.

Thank for your work here!

Rails has multiple database support since 6.0, but as Parity doesn't
parse three-tier database configuration files correctly, it still
doesn't work on Rails applications set up in this way.

This change detects primary configurations. If it finds one, then it
will use it while restoring a backup. Else, it will default to the
first available configuration.
@horacio horacio force-pushed the hb-add-multi-database-support-on-development branch from ac4bfca to ede6e25 Compare October 11, 2021 21:07
@horacio
Copy link
Author

horacio commented Oct 11, 2021

@geoffharcourt thank you for being so generous with your time on my first pull request!

I made the solicited changes. Please let me know if something needs to be changed / reworded on the commit message to make it clearer.

@geoffharcourt geoffharcourt merged commit c903982 into thoughtbot:master Oct 11, 2021
@megatux
Copy link

megatux commented Feb 22, 2022

Is there a new version release with this change included?

@geoffharcourt
Copy link
Collaborator

@megatux v3.5.0 is released!

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.

None yet

3 participants