- 🇺🇸United States mile23 Seattle, WA
Just to note that we do things like this: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...
// Indicate that code is operating in a test child site. if (!defined('DRUPAL_TEST_IN_CHILD_SITE')) { if ($test_prefix = drupal_valid_test_ua()) { $test_db = new TestDatabase($test_prefix); // Only code that interfaces directly with tests should rely on this // constant; e.g., the error/exception handler conditionally adds further // error information into HTTP response headers that are consumed by // the internal browser. define('DRUPAL_TEST_IN_CHILD_SITE', TRUE); // Log fatal errors to the test site directory. ini_set('log_errors', 1); ini_set('error_log', $app_root . '/' . $test_db->getTestSitePath() . '/error.log'); // Ensure that a rewritten settings.php is used if opcache is on. ini_set('opcache.validate_timestamps', 'on'); ini_set('opcache.revalidate_freq', 0); } else { // Ensure that no other code defines this. define('DRUPAL_TEST_IN_CHILD_SITE', FALSE); } }
...when we could do things like:
$kernel = new DrupalKernel('kernel_test', etc);
Of course it's much later in the game than comment #18, approx 12 years ago, which was the time to adopt this model. Risk-averse maintainers probably don't want to untangle it now.
Speaking of which, I think
DRUPAL_TEST_IN_CHILD_SITE
might have survived all the way from Drupal 7, which means evolutionarily speaking it's the fittest. - 🇺🇸United States xjm
@catch, @longwave, @alexpott, @lauriii, @bnjmnm, and I discussed this issue at DrupalCon Barcelona 2024.
Previously on this issue, we discussed the possibility of allowing two values, three values, or an arbitrary number of user-defined values for different site environments.
Today we discussed and agreed that we will restrict this to two modes (dev and prod, or along those lines), for the following reasons:
-
"Staging" or "QA" means different things to different sites and organizations. If we provide this middle option, developers and site owners will make assumptions about what it means in terms of which production behaviors you get. (E.g., that a site in staging won't send email, or that it will but that the mail client will handle the test emails, etc.). These misunderstandings could have serious consequences for site data and operations.
-
It's comparatively easy for a site owner or developer to understand what a site being "dev" means and how that differs from production.
We also discussed where this flag should be stored:
- It should not be in config because it is not something that you deploy.
- It could be stored in either
settings.php
orservices.yml
- We could optionally allow overriding it in state or similar (so that there would be both filesystem and database methods for overriding it).
#75 in particular is out of scope for the dev/prod toggle we're discussing here.
-
- 🇫🇷France fgm Paris, France
Maybe it's bikeshedding, but terms I usually here in this context are a bit more general: rather than "dev"/"prod" the distinction tends to be "build" vs "run", which may be more explicit: it's either an environment in which devs intervene, or one on which they (usually) don't.
Also, as to where, the most logical location for something which is deployment-dependent seems to be settings. The bigger plus is that it can be updated without downtime by a single file change. The minus being that it conditions modules and services availability. "Run" deployments usually have dedicated cache servers, queue servers, proxy, CDN, which may not be enabled on a "Build" environment, and that means differences from the
composer.json
up, with different resulting modules and configurations, which is not something that something in settings can support.The way I've been doing it on client projects since Drupal 8 pre-alpha has always been to expose this in the composer extra information, so it's available from the build step, AND use it to generate two per-environment settings using (Twig) templating, so all scripts and composer plugins run relative to that environment. Deployment commands, being "build", use the "build.settings.local.php" target, while everything after build (hence run) uses the "run.settings.local.php" target, switching being performed because the "settings.local.php" is just a symlink that can be updated atomically.
- 🇬🇧United Kingdom catch
Given we decided this would only be a boolean, I think we could make it
$settings['development_mode'] = TRUE;
(or however mechanism we define it) and then it would really be a development mode toggle, then leave exactly what it means when it's FALSE implicit. - First commit to issue fork.
- 🇩🇪Germany geek-merlin Freiburg, Germany
One objection to a setting: IIRC, settings jump in quite late. A container parameter would be better in this regard.
- 🇩🇪Germany geek-merlin Freiburg, Germany
And a much bigger objection: The MR contains @dawehner's quich shot from #58 from 7 years ago, and is a strong Drupalism.
Let's instead leverage the
kernel.environment
container parameter. It is symfony standard and already heavily supported in our Drupal kernel:
- \Drupal\Core\DrupalKernel::getKernelParameters
- \Drupal\Core\DrupalKernel::getContainerCacheKey - 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium
To be honest I'm fine with keeping this a setting. I don't regard Settings::get() as that bad of a Drupalism, or at the very least don't think this issue should be held up by it.
If we want to move away from using Settings in favor of container parameters, then we need to first agree on that in a separate issue and come up with a plan to remove all use of Settings. But then you'd have to inject the container into every single service that might need to check for a setting. There's also a difference between what you can do in a settings.php file and a services.yml file.
As you can see, many things left to discuss on that front. So maybe let's be fine with adding "yet another" setting here and save that discussion for another issue?
Will rerun the pipeline as there seem to be random failures. Keeping at NW for a hook_requirements test, but that should be straightforward.
- 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium
Can't seem to rerun the pipeline for this MR (got push access).