Skip to content

Ensure that the loaded config survives an application reload#1

Open
tompave wants to merge 1 commit intomrluc:masterfrom
tompave:persist_the_config
Open

Ensure that the loaded config survives an application reload#1
tompave wants to merge 1 commit intomrluc:masterfrom
tompave:persist_the_config

Conversation

@tompave
Copy link

@tompave tompave commented Jul 17, 2017

Pass persistent: true to Application.put_env/4, so that the new config values will survive the application load and reload events.

Documented here. As example, it's used by Mix.Config.persist/1.

…fig will survive application load and reload.
@tompave tompave changed the title Pass 'persistent: true' to Application.put_env/4, so that the new con… Ensure that the loaded config survives an application reload Jul 17, 2017
@mrluc
Copy link
Owner

mrluc commented Jul 17, 2017

That makes perfect sense to me as default behavior; I want it. But I'd like to make sure I think it through, because it seems like this could be breaking to some people.

Could you describe your use case?

I ask because we're already encouraging people to use DeferredConfig from the Application.start callback, which means that the app shouldn't be missing config due to an ordinary app stop/start, so is the use case related to hot code reloading and not getting overridden by eg. app resource files?

@tompave
Copy link
Author

tompave commented Jul 17, 2017

Hi, thanks for the quick reply.

To be honest I don't have a use case for this. I'm using this package for a project I'm working on, I was reading the sources and I noticed that no option was passed to Application.put_env/4. It's working great in development for the moment. 👍

I just thought that this makes sense because I imagine that — as you mentioned — with the current implementation hot-code-reloading would wipe out the dynamic configuration values.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants