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

Improve the install generator to handle two- and three-tier database.yml config files #206

Closed

Conversation

fractaledmind
Copy link
Contributor

Resolves #204

@dhh: I'm not claiming this code is lovely, but I wanted to get a working implementation done quickly so that we can iterate on the code itself. Here are the key details:

  • load the YAML into a Ruby hash, resolving aliases, to get a clear picture of the configuration without resorting to weird string checks
  • work with the entire environment definition block so that find and replace can handle comments and additional config values seamlessly
  • have a fallback if we can't make sense of the file (I just append a comment with the contents that the user needs to integrate into the config file)
  • works with whatever indention the user currently has (tabs or spaces)
  • distinguishes between two and three tier configurations based on the present (or not) of the primary key in the configuration hash

I worked with a number of example database.yml files while testing the basic logic myself. Happy to add some test cases for any states you want to cover. If I were to lay out the basic matrix, it would be

  • sqlite vs other
  • 2-tier vs 3-tier
  • cache db not present vs cache db already present

Your tests cover

  1. sqlite, 2-tier, cache db not present
  2. other, 3-tier, cache db already present

I can add any combinations you think are highest value.

@dhh
Copy link
Member

dhh commented Sep 4, 2024

Will review. But starting to think that maybe we should just have the installer spit out a "add this to production:" if the default file is anything but vanilla, and then folks can just add it themselves. Definitely a drawback of using configuration files that are meant for human editing!

@dhh
Copy link
Member

dhh commented Sep 4, 2024

In fact, maybe the right option here is actually to do the solid preconfiguration straight in Rails, so that delivers both the out of the box configuration and enables us to use db:system:change without problems. Then the installer can simply ask the user to add this block themselves when run directly.

@fractaledmind
Copy link
Contributor Author

I think that would prove much simpler in the long run. As you can see, mutating the database.yml file is just a deep problem space.

And given that Rails will own app generation and know whether Solid Cache/Queue is to be included, it can just spit out the appropriate database.yml. And this installer, as you said, becomes much dumber and just provides instructions on how to setup the database, if this installer is run independently of generating a Rails app.

@dhh
Copy link
Member

dhh commented Sep 4, 2024

Solved this via rails/rails@cae25f3 and 0a36dec. Appreciate you looking into this other approach! ✌️

@dhh dhh closed this Sep 4, 2024
@fractaledmind
Copy link
Contributor Author

Excellent. Both look great.

@fractaledmind fractaledmind deleted the improved-db-yml-installer branch September 4, 2024 17:53
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.

Improve the installer to account for an existing multi-existing db configuration
2 participants