Open cjlarose opened 6 months ago
I have two suggestions about interface of the feature functionality:
Convention Over Configuration Approach: Enforce a standardized file organization for configurations. For example, files should be loaded in the following sequence:
settings.yml
).settings/default/separated_config.yml
, settings/default/subdir/another_separated_config.yml
).settings/production.yml
).settings/production/separated_config.yml
, settings/production/subdir/another_separated_config.yml
).
Potential Risk: If users already have some files in settings/default
, they might encounter issues after updating the configuration gem. The new version could attempt to process these files, leading to unexpected behavior.Extending Configuration:
Provide a mechanism to modify the order of configuration files loaded by the Config.process_settings_files
method:
Config.setup do |config|
config.process_settings_files do |settings_files|
# Add files to the beginning of the list
settings_files.unshift('config/settings/default/separated_config.yml')
# Append files to the end of the list
settings_files << 'config/settings/default/another/separated_config.yml'
# Or redefine settings_files
settings_files = ['some_config.yml']
settings_files
end
end
This approach offers flexibility in configuring the loading order, ensuring users can prioritize their configuration files as needed.
I can try to prepare PR
I think you're hitting on all of things that are important to consider
config/development.yml
for example if loading Rails in development mode. Conversely, when loaded outside of Rails, users are expected to be explicit about which files are loaded (they must call Config.load_and_set_settings
themselves)I think the idea of being able to have control over the order over which files are loaded, and I think the suggested API for config.process_settings_files
would make sense for Rails projects, where there is essentially a "default" set of settings files. For non-Rails use cases, though, it doesn't really make sense to be passed the "default" set of settings files since there really isn't one. I think I'd be more inclined to have something where the user just gives the explicit list of files (either as an Array
or as a block that yields an Array
)
Config.setup do |config|
config.default_settings_sources = [
"config/settings.yaml"
]
end
The default value for the new config.default_settings_sources
setting can just be empty. Or if you're in a Rails project, it should be set to basically Config.setting_files(::Rails.root.join('config'), ::Rails.env)
. If they want to express their list of settings files by manipulating the default list of files programmatically, they can by calling Config.setting_files
explicitly.
Config.setup do |config|
config.default_settings_sources = do
settings_files = Config.setting_files(::Rails.root.join('config'), ::Rails.env)
settings_files.unshift('config/settings/default/separated_config.yml')
settings_files << 'config/settings/default/another/separated_config.yml'
end
end
What do you think?
Thinking about it a bit more, I guess both approaches depart from what I think is the original design of the gem:
Config.setup
is used configure the behavior of the gem (without actually specifying the literal list of files).Config.load_and_set_settings
actually takes in the list of files. For non-Rails projects, this must be called explicitly. In Rails projects, this is called automatically in the Railtie. In fact, calling it from config/initializers/config.rb
will actually be ignored.This distinction is interesting to me because Config.setup
is used to specify things that are not related to the list of files to load like how merging settings hashes should behave and which constant to use (by default, Settings
), but also things that are definitely related to where settings should come from (e.g. use_env
and, more recently, file_name
and dir_name
). file_name
and dir_name
, I think exist largely because in a Rails project, the user doesn't have a lot of control over the actual list of settings files. Those two options only really have an effect when calling Config.setting_files
.
I'm not 100% bought-in that this present distinction between config and data (the actual list of files) is the perfect API, but I also want to respect the precedent and I think being able to specify the list of settings files in both Config.setup
and Config.load_and_set_settings
would be confusing.
Therefore, I'm thinking the most straightforward solution to this problem is to just make it so that if a Rails user calls Config.load_and_set_settings
in config/initializers/config.rb
, then the Railtie shouldn't also call the same function (and thereby essentially overwrite the settings loaded in the initializer). The API for Config.load_and_set_settings
already gives users the ultimate flexibility in what files are loaded. The actual problem, I think, is just that the Railtie clobbers any use of that method by doing its own Config.load_and_set_settings
.
So for Rails uses that are completely bought in to the default list of settings files, they don't have to call Config.load_and_set_settings
themselves. But for Rails users that actually do want to customize the list of files, they are welcome to by calling Config.load_and_set_settings
in the initializer with whatever list of files they like.
I understand your position and agree that there is a reason to stick to the original plan.
I like the idea of having Railtie not call Config.load_and_set_settings
if the user has already called it before.
I tried to think about how this could break backward compatibility, but I couldn't find any issues. reload_from_files
, which is called at runtime but later than the initial initialization, so it seems there shouldn't be any problems with it. If users, for some reason, called Config.load_and_set_settings
earlier, Railtie still did not take this into account.
Then the project's config might look like this:
# app/initializers/config.rb in a Rails project
Config.setup do |config|
Config.load_and_set_settings(file_list)
end
And if the configuration is already defined (load_and_set_settings
has already been loaded), then we do not perform the reloading.
class Railtie < ::Rails::Railtie
def preload
...
return if Object.const_defined?(Config.const_name)
# Parse the settings before any of the initializers
Config.load_and_set_settings(
Config.setting_files(::Rails.root.join('config'), ::Rails.env)
)
...
Yeah, that's perfect. Would you be interested in opening a PR?
Yes, I will try
https://github.com/rubyconfig/config/pull/353
I'm not found a way to test it yet.
I need to first execute the Config.Setup
code, and after that it should execute Config::Integrations::Rails::Railtie.preload
But all specs runs after Config::Integrations::Rails::Railtie.preload
. So we can not test it with standard bundle exec appraisal rails...
Yeah, there's not a good existing test pattern to use for this case. There are a few Rails apps in spec/app
, but currently we just run the tests after executing require_relative "app/#{app_name}/config/environment"
. That's useful for running the complete test suite under the environment of a particular Ruby/Rails version combination, but indeed this "pollutes" the test environment if we're trying to test something about the Rails boot sequence itself.
Without completely reorganizing the test suite, the most straightforward approach I see is to test the Railtie behavior is something like
Rails.root.join('config', 'initializers', 'config.rb')
Rails.root.join('config', 'settings.yml')
Settings
based by interacting with the current Rails app via the CLI (e.g. ./bin/rails runner 'puts Settings.expected_key'
). This way, the Rails app will be loaded in a realistic way (the Railtie should run, then the initializers as normal) and it isn't polluted by the fact that the test process itself has already loaded the Rails app.We can rinse and repeat this pattern for a few tests
config.rb
, the default setting files are loaded (anything in config/settings.yml
for example)config.rb
, but without any use of Config.load_and_set_settings
, the desired settings apply (for example file_name
or dir_name
), and the same default setting files are still loaded (if file_name
is set to config
, then config.yml
is loaded)config.rb
that calls Config.load_and_set_settings
, the desired settings apply and only the specified files are loaded (and not any other yml files like config/settings.yml
)Thank you for great advice! I added tests
In Rails projects,
config/initializers/config.rb
can be used to configure this gem, but the Railtie performs the actual first load of the settings files. The Railtie doesn't offer a way to specify additional settings sources (for example, multiple YAML files).It's possible to specify additional settings files today if users use a separate initializer, but this isn't always convenient because it essentially means that settings are loaded twice (and both times must satisfy the configured schema or contract) and because the initializer runs much later in the Rails boot process (so it's not available when evaluating some files like
config/environments/production.rb
)Related: