-
-
Notifications
You must be signed in to change notification settings - Fork 539
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 DB provider adapter config & sensible defaults #1430
Conversation
86db090
to
d92d331
Compare
d92d331
to
0bf0697
Compare
3aaded8
to
6399e4c
Compare
…ay is a user-provided config
@configured_for_database = false | ||
end | ||
|
||
def finalize_config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is pulled out into a public method on the provider source so we can call it on the parent provider in #apply_parent_config
. Doing it this way is important to avoid unnecessary connections to the database that would otherwise occurred if we called a straight-up #prepare
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great ❤️
I think this covers any customizations I would need to do.
4d06c60
to
5ca573b
Compare
Note to reviewers: I'm 99% confident this is a good approach and should work well for us. I'm interested in your perspectives on the config API here. Have I missed anything?
Add adapter config and sensible adapter defaults to the DB provider.
Now you can configure your adapters like so:
The following defaults are provided for the :sql adapter:
And in addition for Postgres databases:
These defaults are set as part of the db provider's
prepare
lifecycle step. This is necessary (instead of doing it earlier, i.e. at the stage of establishing the setting defaults) to allow the user to configure and set theirdatabase_url
, which is required to determine the set of defaults to provide.When configuring the db provider, the defaults may be skipped by the user:
If you want to configure the db provider to use a non-sql adapter, then you can configure it independently:
You can also configure ROM plugins for any adapter via
any_adapter
:This may be useful in an advanced setup where your app uses different ROM adapters, but you want certain plugins to be used consistently across all of them.
For db providers in slices, when using the default
app.config.db.configure_from_parent
arrangement (true
), adapter config is applied from the parent slice just like any other config.However, if the configures their own
:sql
adapter in a slice provider (either plugins or extensions), then the respective plugins or extensions from the parent will not be applied. We might be able to provide more sophisticated handling here, but this felt like a reasonable first approach. In a Hanami app with multiple slices but a single shared database, I think it would be unlikely that different ROM plugins or Sequel extensions would be needed on a slice-by-slice basis anyway.This change depends on dry-rb/dry-configurable#164 to allow for the custom config class we use for the db provider. I'm going to make sure that PR merges first before this one, but I wanted to open this one up for review first.
Resolves #1389, resolves #1409