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

INI shouldn't be loaded from same directory as code #149

Open
NattyNarwhal opened this issue Mar 1, 2021 · 5 comments · May be fixed by #161
Open

INI shouldn't be loaded from same directory as code #149

NattyNarwhal opened this issue Mar 1, 2021 · 5 comments · May be fixed by #161

Comments

@NattyNarwhal
Copy link
Collaborator

Reasoning:

  • If Toolkit is managed by a package manager, then it shouldn't put config in a library directory (confusing, hard to manage)
  • Cases where there should be site-specific configuration (i.e an INI per site)
    • This could be done through say, an environment variable, or in code. The former seems preferable because then it wouldn't require code changes.
@alanseiden
Copy link
Collaborator

alanseiden commented Mar 1, 2021

Thanks. I was thinking about this today while we taught a client how to turn off debug mode via toolkit.ini.

@NattyNarwhal
Copy link
Collaborator Author

proposed search order and names:

  • Environment variable (call it like PHP_TOOLKIT_INI), so it can be overriden per-site if need be
  • /QOpenSys/etc/php-toolkit.ini (for IBM i)
  • /etc/php-toolkit.ini
  • The Toolkit install dir (for compat)

Should it be overrriden in code too?

@NattyNarwhal
Copy link
Collaborator Author

realizing this could be from code too for i.e app specific

@alanseiden
Copy link
Collaborator

Toolkit install dir could contain a symlink to new location

@NattyNarwhal
Copy link
Collaborator Author

I will say that CONFIG_FILE is theoretically settable by defining it before Toolkit.php is loaded, but this seems greasy in a world where autoloaders exist. The fact the INI is basically handled by static resources sucks, and that could require a refactor to make instance-local.

NattyNarwhal added a commit that referenced this issue May 31, 2021
Not sure is this should be it exactly, but per-site configuration and
system-wide (i.e managed by RPM) are common use cases; there may be
more. We should accomodate these when possible, because user
configuration in a source code directory is counter-intuitive and
inflexible.

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

Successfully merging a pull request may close this issue.

2 participants