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

Improve support for settings.local.php in generated configuration #276

Closed
xurizaemon opened this issue Mar 29, 2023 · 2 comments
Closed

Comments

@xurizaemon
Copy link

xurizaemon commented Mar 29, 2023

Drupal supports a settings.local.php override intended to support local overrides to configuration in settings.php.

/**
 * Load local development override configuration, if available.
 *
 * Create a settings.local.php file to override variables on secondary (staging,
 * development, etc.) installations of this site.
 *
 * Typical uses of settings.local.php include:
 * - Disabling caching.
 * - Disabling JavaScript/CSS compression.
 * - Rerouting outgoing emails.
 *
 * Keep this code block at the end of this file to take full effect.
 */
#
# if (file_exists($app_root . '/' . $site_path . '/settings.local.php')) {
#   include $app_root . '/' . $site_path . '/settings.local.php';
# }

ISLE buildkit update_settings_php configuration is placed last in generated settings.php via echo ... >> settings.php.

Calling make up in ISLE DC will call update_settings_php each time:

https://github.com/Islandora-Devops/isle-dc/blob/4f5bebcd12ed3aa3145b57f4cd7774c60d9f806f/Makefile#L258-L266

I wondered if drush islandora:settings:* calls support "upserting" configuration in settings.php, in which case I'd expect moving the block to permit the file order to persist. It does look like the code might support persisting locations in that file if the settings block was moved after generation?

/**
* Determine which settings file to update.
*/
private function writeSettings($settings)
{
require_once DRUPAL_ROOT . '/core/includes/install.inc';
$settings_file = $this->getSettingFilePath();
$fs = new Filesystem();
$fs->chmod($settings_file, 0755);
new Settings([]);
drupal_rewrite_settings($settings, $settings_file);
$fs->chmod($settings_file, 0400);
}
}

Templating the configuration in so that it appears before the inclusion of settings.local.php would support environment-specific overrides to settings.php when update_settings_php is called after initial build.

@nigelgbanks
Copy link
Contributor

nigelgbanks commented Mar 29, 2023

I would like to push folks to not modify settings.php at container startup, but rather take the approach used in the isle-site-template. That reads the settings passed in via the environment or secrets. Ideally, these custom drush commands will get removed from isle-buildkit. The plan is to deprecate them for this next release and remove them in the next after that.

For example this line takes its value from the DRUPAL_DEFAULT_BROKER_URL environment variable.

This technique should be used with the drupal-scaffold feature of composer to append some text on to default.settings.php and write out to settings.php on composer install, such that it's baked into the Docker image. As is done in the starter site here

@xurizaemon
Copy link
Author

xurizaemon commented Mar 30, 2023

That looks much better.

I'll leave this issue open on the assumption that Isle Buildkit is still under development, in case the proposal has merit here. If isle-site-template is the path ahead, happy for this issue to be closed.

Thanks @nigelgbanks!

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

2 participants