Rails integration does not allow multiple YAML files to be specified in the first load

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)

Convention Over Configuration Approach:
Enforce a standardized file organization for configurations. For example, files should be loaded in the following sequence:

Basic Configuration: Always loaded first (settings.yml).
General Configuration Files: Placed in a default directory and applied across all environments (settings/default/separated_config.yml, settings/default/subdir/another_separated_config.yml).
Environment-Specific Configuration: Overrides for specific environments (settings/production.yml).
Overloading Configuration Files: Allows further customization within specific environments (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

Rails users, broadly, are accustomed to convention-over-configuration. The default Railtie already loads 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)
Ideally, changes would be made an a backward-compatible way. Loading additional paths by default in a version bump is probably fine, but removing them should be avoided

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

#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

Write a new file to Rails.root.join('config', 'initializers', 'config.rb')
Write any necessary YAML files e.g. Rails.root.join('config', 'settings.yml')
Make assertions on 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.
Delete the temporary files

We can rinse and repeat this pattern for a few tests

Without any config.rb, the default setting files are loaded (anything in config/settings.yml for example)
With a 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)
With a 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

Navigation Menu

Repository navigation

Rails integration does not allow multiple YAML files to be specified in the first load #352

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Rails integration does not allow multiple YAML files to be specified in the first load #352

Description

Activity

vovanmozg commented on Mar 2, 2024

cjlarose commented on Mar 6, 2024

cjlarose commented on Mar 6, 2024

vovanmozg commented on Mar 6, 2024

cjlarose commented on Mar 6, 2024

vovanmozg commented on Mar 6, 2024

vovanmozg commented on Mar 6, 2024

cjlarose commented on Mar 7, 2024

vovanmozg commented on Mar 7, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions