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

How do I dynamically modify the configuration? #2214

Closed
leeqvip opened this issue Sep 12, 2019 · 11 comments
Closed

How do I dynamically modify the configuration? #2214

leeqvip opened this issue Sep 12, 2019 · 11 comments

Comments

@leeqvip
Copy link
Contributor

leeqvip commented Sep 12, 2019

I tried to modify the configuration hostname, it doesn't work:

$config = config('Database');
// echo $config->default['hostname']; // 127.0.0.1
$config->default['hostname'] = '192.168.1.2';

Then in another place:

$config = config('Database');
// echo $config->default['hostname']; // 192.168.1.2

Database::connect('default');

It still uses 127.0.0.1 to connect ...

@MGatner
Copy link
Member

MGatner commented Sep 12, 2019

I believe this is a duplicate request to #1661

@jim-parry
Copy link
Contributor

It is similar to the one you mention, as well as #1618, but different ... wanting to change configuration settings at runtime, from the sound of it. It could be that addressing wither of the others in 4.1 might resolve this too.

@leeqvip
Copy link
Contributor Author

leeqvip commented Sep 16, 2019

Can we change new \Config\Database() to config('Database') ?
such as: system/Database/Config.php#L90

@lonnieezell
Copy link
Member

@techoner changing that line is a good idea, I think. Care to submit a PR?

@leeqvip
Copy link
Contributor Author

leeqvip commented Sep 16, 2019

@lonnieezell Just submitted a PR, #2224

@lonnieezell
Copy link
Member

@techoner Awesome. Thanks! Merged.

Does this fix this issue also?

@MGatner
Copy link
Member

MGatner commented Sep 16, 2019

This brings up a good point, now that we have config($serviceName) that searches across namespaces in an appropriate order (App, System, third party) should internal config calls be using the function (or Config\get()) instead of the class directly to enable overriding/extending?

@MGatner
Copy link
Member

MGatner commented Sep 16, 2019

... followup question, should the "appropriate order" be (App, third-party, System) to enable modules to override core configurations? Increases security concerns but also extension potential.

@lonnieezell
Copy link
Member

@MGatner Yes, internally we should typically be using either the helper or Config\get() since that caches the results and helps keep things speedy. I a partial sweep through the framework when i created that functionality, but obviously was not an exhaustive one.

There might be occasions where we don't want to use that but I think 99% of the time it should use it.

@lonnieezell
Copy link
Member

@MGatner as for "appropriate order" my decision on that order the first time put system above modules to ensure someone couldn't install a module they hadn't inspected very well and run into a situation where the modules' config overrode system settings without the app realizing it, potentially leading to bad results. Seems safer to this way.

If they want to use a config file that's named the same as one in the system, it should be a known step on the part of the developer to copy it from the module to the App/Config.

@leeqvip
Copy link
Contributor Author

leeqvip commented Sep 17, 2019

This fixes this issue.

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

No branches or pull requests

4 participants