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

Compability with Rails engine #323

Open
alienxp03 opened this issue Aug 5, 2022 · 3 comments
Open

Compability with Rails engine #323

alienxp03 opened this issue Aug 5, 2022 · 3 comments

Comments

@alienxp03
Copy link

I'm trying to use the gem with Rails engine, and noticed there might be some issue.

# Assume ENGINE_ROOT = /home/user/new_engine
# in engine.gemspec
s.add_dependency 'config'

# Common step for Rails engine. Add this code to ENGINE_ROOT/lib/new_engine/engine.rb
require 'config'

# Run command from the engine's root path
 > bundle install
> rails g config:install # all files are generated correctly in ENGINE_ROOT/config

Pretty straightforward setup so far. However adding any new settings doesn't load by default, which is odd. For example:

# ENGINE_ROOT/config/settings/settings.development.local
total: 10

# In Rails console
> Settings.total 
nil

> Settings.add_source!('config/settings/test.local.yml')
=> [#<Config::Sources::YAMLSource:0x0000563cbe0bbde0 @evaluate_erb=true, @path="/home/user/new_engine/spec/dummy/config/settings.yml">,
 #<Config::Sources::YAMLSource:0x0000563cbe0bbd68 @evaluate_erb=true, @path="/home/user/new_engine/spec/dummy/config/settings/test.yml">,
 #<Config::Sources::YAMLSource:0x0000563cbe0bbd18 @evaluate_erb=true, @path="/home/user/new_engine/spec/dummy/config/environments/test.yml">,
 #<Config::Sources::YAMLSource:0x0000563cbe0bbcf0 @evaluate_erb=true, @path="/home/user/new_engine/spec/dummy/config/settings/test.local.yml">,
 #<Config::Sources::YAMLSource:0x0000563cbe0bbcc8 @evaluate_erb=true, @path="/home/user/new_engine/spec/dummy/config/environments/test.local.yml">,
 #<Config::Sources::YAMLSource:0x0000563cbe2db6c0 @evaluate_erb=true, @path="config/settings/test.local.yml">]

Adding the source manually works, but what I noticed here is that Settings is trying to load the default settings files from the wrong location.

Correct path: /home/user/new_engine/config
Current path being used: /home/user/new_engine/spec/dummy/config.

Some background. In Rails engine, we can create a dummy application so that it's easier for us to test it out. Usually the path is ENGINE_ROOT/spec/folder_name. From what I noticed so far, it looks like Settings is trying to load the config from the dummy path not the engine's root path.

I'm just guessing at the moment, but it could be because Settings is using Rails.root to find the right default path. This should work for normal Rails app. However for Rails engine, Rails.root is referring to the dummy path hence the issue. In Rails engine, typically we want to use NewEngine::Engine.root for the right engine's path.

Hope all is clear so far. Based on this, I'm wondering if we could make the gem work by default with Rails engine. So far I can see two options that we can do with minimal changes to the gem.

  1. I'm not sure how, but instead of using Rails.root as the path, we can use NewEngine::Engine.root. instead. However, since every engine have different name, I'm not so sure on how can we accomplish this.
  2. I'm looking the default template. I'm thinking whether we could add another option for the developer to set/override the default root path. So this way, we can use Rails.root by default. And for Rails engine, I can manually override this path to use NewEngine::Engine.root instead.

Do let me know if I'm missing anything, or if there's anything that's not clear.

@pkuczynski
Copy link
Member

@cjlarose wdyt?

@Fryguy
Copy link
Member

Fryguy commented Aug 5, 2022

We also use engines in ManageIQ, but our use case is a little different. Basically we use Rails engines as a plugin system, and one of the pluggable things is the config settings. These engines can bring their own settings.yml and we will add those to the set of YAML Sources.

To do this we built an initializer that essentially loads config from all of our engines and the main app, all in the expected override order, then replaces the Settings constant. You can see the code here and follow along: https://github.com/ManageIQ/manageiq/blob/0a1c16529c96eb5367ed311e08fcd881f94cc818/lib/vmdb/settings.rb#L30, but tl;dr we loop through all of the engines in our application and add YAML Sources for each of them.

Ideally, config should more or less just work with engines and ManageIQ shouldn't have to have a custom initializer. I'm not sure if they should automatically work with engines by detecting settings.yml files, or if it makes sense to have a Config parameter where you opt in to which engines you want to honor settings from.

The other question is which way does priority work? If a key appears in the main application's settings and an engine's settings, which one wins? In ManageIQ our priority is that engines override the main application, but I'm not sure that same decision applies outside of our specific application or not.

@alienxp03
Copy link
Author

Ideally, config should more or less just work with engines and ManageIQ shouldn't have to have a custom initializer

Agree with this part. At the moment I can customize the code on my engine to make it work. Not a big issue. But would be nice if it could just work with Rails engine, since from the engine's perspective, it's just another gem.

At the moment I'm not sure either on the solution, or the gem's direction with Rails engine. Just opened an issue here in case others might face the same issues that I have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants