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

Added config.prepend_sources option to Config.setup #142

Closed
wants to merge 7 commits into from

Conversation

tjgfernandes
Copy link

No description provided.

@pkuczynski
Copy link
Member

@tjgfernandes, could you please add some documentation explaining this new feature? I honestly see no use case, so I am interested to see how you present it...

@tjgfernandes
Copy link
Author

@pkuczynski, need to prepend custom yml setting file that depends on ENV variable to be sure that all settings dependencies are loaded based on that custom yml file. This way settings just run once, for me it's more clean/clear than using runtime approach. Also, any initializer i have can use Settings.

This way you can choose to prepend_source at config time or runtime.

@pkuczynski
Copy link
Member

So the idea you have is to be able to load another settings file before loading current set of files based on the current environment? Sorry for asking, but why would you like to do that? More natural feels: load default settings and then override some of them with custom file based on ENV variable...

Could you please elaborate a little bit more on this, as I would like to better understand the reasoning before merging this into master.

@tjgfernandes
Copy link
Author

@pkuczynski i see your point but don't see it as more natural. config.prepend_sources allows to have pre custom settings without worring to create another initializer to add_and_reload source. Configuration is done is one place and it runs once. Don't need to worry to what happens with Rails app and settings usage/dependencies in between Config initialize and source_added_and_reloaded! (even for a very short time). Settings may be updated on reload! but what happens with any config you might have passed the previous Settings value inside the Rails app?

note: The other way to do this would be to allow to config settings files at config time, but didn't have that need, settings.yml file and others are fine has they are.

@tjgfernandes
Copy link
Author

@pkuczynski, have pull request this because i was thinking it might be a small change that could be useful. But if this isn't something you see usefull for railsconfig users, please don't merge. No problem on my side!

@pkuczynski
Copy link
Member

I see your point, but I am not sure about the implementation. Let me think about it for a while and abstract it in a more general way, where you can update which files are loaded in which order in a simple and clean way. I do not like source_added_and_reloaded! myself and was planing to refactor this in the nearest future...

@3den
Copy link

3den commented Mar 29, 2017

@pkuczynski any update on this?

This feature is very useful for my team, coz I work on apps with lots of yaml settings, that most of the time are not environment based, so we break up the settings into smaller yaml files e.g. settings/faq.yml, settings/swagger.yml, settings/data_table.yml...

Ideally we should also have an append_sources method.

@pkuczynski
Copy link
Member

Apologies. I will have a look tomorrow.

@vovanmozg
Copy link

I have the same problem as 3den. Our settings.xml file already contains over 2000 lines.
I like the modular way of organizing configuration, like nginx.
It's convenient to be able to use separate yaml files.

@cjlarose
Copy link
Member

cjlarose commented Mar 1, 2024

This PR is old enough that I think it's probably best to close it. I think the concern is still valid though. I opened #352 to track it.

@cjlarose cjlarose closed this Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants