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

Change database yml directly and double down on separate DB #202

Merged
merged 9 commits into from
Sep 3, 2024

Conversation

dhh
Copy link
Member

@dhh dhh commented Sep 2, 2024

For Solid Cache to become a Rails 8 default, the configuration needs to be fully automated, such that we can run it as part of "rails new". This PR makes the installation a single step process, and commits to using a separate database for the default case.

This also converts the setup from using a migration to providing the full SC db as a schema. This is needed in order to only have the cache db configured in production. Otherwise you also need it configured in development, even if you're not using it, or the migration installation won't run. It's better anyway like this.

@dhh dhh requested a review from djmb September 2, 2024 22:59
dhh added 5 commits September 2, 2024 16:15
And fix the DATABASE env name
These files are required in all Rails apps. If they are missing, it's
fine to explode.
Comment on lines +15 to +19
def add_cache_db_to_database_yml
if app_is_using_sqlite?
gsub_file database_yml, /production:\s*<<: \*default.*/m, sqlite_database_config_with_cache
else
gsub_file database_yml, /production:\s*<<: \*default.*/m, generic_database_config_with_cache
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how you want to handle this situation, but if someone is adding Solid Cache to an application where they have already added, for example, Solid Queue—and thus their database.yml is already 3-tier, this won't add the cache database to their config.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this needs to be more resilient. If you're interested, please do take a look to make it so! We want Solid Cache and Solid Queue configured out of the box and with sqlite by default, so this does need to be resolved.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to release with this for now to move the ball forward on integration. But we will have to revisit this again when we have a fix so we can get solid queue in on the same deal.

Co-authored-by: Stephen Margheim <stephen.margheim@gmail.com>
@dhh dhh merged commit f86ac0a into main Sep 3, 2024
19 checks passed
@dhh dhh deleted the change-database-yml branch September 3, 2024 23:03
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.

5 participants