Use config override instead of event subscirber to enable the profile split

Created on 3 February 2022, almost 3 years ago
Updated 31 January 2024, 11 months ago

Problem/Motivation

subscribing to the KernelEvents::REQUEST in order to change the global $config variable is not a good idea.
It doesn't work with drush and is in general not a good approach.

Proposed resolution

Switch to an approach with a service implementing ConfigFactoryOverrideInterface.

See Config Split 2.x for a simple example:
https://git.drupalcode.org/project/config_split/-/blob/2.0.x/src/Config/...

and
https://git.drupalcode.org/project/config_split/-/blob/2.0.x/config_spli...

The service in config split is also used to change the setting via state, in this module here you don't need to do that, and you also don't need to invalidate any caches because one can not change the install profile.

Remaining tasks

patch, test, commit, celebrate

User interface changes

none

API changes

none

Data model changes

none

πŸ› Bug report
Status

RTBC

Version

2.0

Component

Code

Created by

πŸ‡¨πŸ‡­Switzerland bircher πŸ‡¨πŸ‡Ώ

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • I tested this one with D10 and profile_split_enable 2.0.1. Without this change, the profile enable split does not work at all. Thank you!

  • Status changed to Needs work 11 months ago
  • πŸ‡ΊπŸ‡ΈUnited States Chris Burge

    This MR adds tests, and GitLab automated tests aren't configured for this project. Locally, they pass, but only after I install the config_split module:

    $ ../vendor/phpunit/phpunit/phpunit -c core/ modules/contrib/profile_split_enable/ --verbose
    PHPUnit 9.6.15 by Sebastian Bergmann and contributors.
    
    Runtime:       PHP 8.1.16
    Configuration: /var/www/html/web/core/phpunit.xml.dist
    
    Testing /var/www/html/web/modules/contrib/profile_split_enable
    ....                                                                4 / 4 (100%)
    
    Time: 00:05.004, Memory: 4.00 MB
    
    OK (4 tests, 5 assertions)
    

    Without config_split:

    $ ../vendor/phpunit/phpunit/phpunit -c core/ modules/contrib/profile_split_enable/ --verbose
    PHP Fatal error:  Trait "Drupal\Tests\config_split\Kernel\SplitTestTrait" not found in /var/www/html/web/modules/contrib/profile_split_enable/tests/src/Kernel/Config/StatusOverrideTest.php on line 9
    
    Fatal error: Trait "Drupal\Tests\config_split\Kernel\SplitTestTrait" not found in /var/www/html/web/modules/contrib/profile_split_enable/tests/src/Kernel/Config/StatusOverrideTest.php on line 9
    

    Is config_split just a missing dev dependency or is a hard dependency for this module?

  • πŸ‡ΊπŸ‡ΈUnited States Chris Burge

    Based on this module's README, it appears that config_split is optional:

    When using config_split it allows you to dynamically enable and import
    configuration from a config split for each of your custom installation
    profiles.

  • Status changed to Needs review 8 months ago
  • πŸ‡ΊπŸ‡ΈUnited States Chris Burge

    I just added config_split as a test dependency to the merge request.

  • πŸ‡ΊπŸ‡ΈUnited States orbmantell

    I came across this issue, and the changes here (mostly) resolved the issue I was having (drush not being able to import profile-based splits)

    The issue is mostly resolved because the referenced branch here is behind 2.x (https://git.drupalcode.org/project/profile_split_enable/-/compare/326217...). I created a patch locally to merge these changes, but was unable to get the test_dependencies change to apply cleanly. When I removed that change the patch applied and my issue was resolved.

    If the changes in this ticket incorporate the latest changes from 2.x and the test_dependencies change is able to apply cleanly my vote is to get this merged. I am leaving this ticket as needs review since there is no patch that applies cleanly, but from a "the change fixes the issue" standpoint I would say this is RTBC.

  • Yes, unfortunately modules doesn't work as expected using drush.

    The fix from MR as a patch for branch 2.x is attached.

Production build 0.71.5 2024