Issue #3220627: Optionally apply configuration changes automatically

[mstenta]

See https://www.drupal.org/project/farm/issues/3220627

[paul121]

This runs every time the cache is rebuilt? eg: drush cr ? And this was the simple way of getting this to run via update.php?

@mstenta I think you may have commented on this elsewhere or in chat, forgive me if that's the case!

I could see this lead to some issues during development - for example when editing & debugging the core asset/log/plan views. But maybe the solution is to just disable farm_update while doing this type of work?

[paul121]

Ahh, from this comment: https://www.drupal.org/project/farm/issues/3220627#comment-14224994

Alternatively, I wonder if hook_cache_flush() would work? Does that run every time that update.php is run? If so, then all we have to do is tell users to run update.php and it will update the config. But... I'm not sure if caches get cleared by update.php if there aren't any actual hook_update_N() implementations to run...

So I'm assuming hook_cache_flush() didn't work?

Interesting there isn't way to run hook_post_update_NAME() after each update 🤔

[mstenta]

This runs every time the cache is rebuilt? eg: drush cr ?

Correct - so there are two ways to run this via Drush: drush cr (to rebuild all caches and revert config) and drush farm_update:rebuild (to only revert config).

And this was the simple way of getting this to run via update.php?

Unfortunately, this still only runs via update.php if there are other updates to be performed. If not, then caches are not rebuilt.

So we may want to consider adding a step to the update instructions for explicitly clearing caches after updates.

I could see this lead to some issues during development - for example when editing & debugging the core asset/log/plan views. But maybe the solution is to just disable farm_update while doing this type of work?

Agreed. And yea that was my thought: disable farm_update.

So I'm assuming hook_cache_flush() didn't work?

I think hook_cache_flush() would also work, but the documentation for that hook states:

Do NOT use this hook for rebuilding information. Only use it to flush custom caches.
...
This hook is invoked by drupal_flush_all_caches(). It runs before module data is updated and before hook_rebuild().

So hook_rebuild() seemed like the better place for this.

Interesting there isn't way to run hook_post_update_NAME() after each update 🤔

Yea agreed! I wish there was a simple hook_post_update() that always ran after update.php or drush updb. That would keep our update instructions simpler, and wouldn't require an additional step for this.

[mstenta]

Hmm - I just found two hooks that DO fire with update.php:

• hook_update_last_removed(): https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Extension%21module.api.php/function/hook_update_last_removed/9.2.x
• hook_removed_post_updates(): https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Extension%21module.api.php/function/hook_removed_post_updates/9.2.x

Neither of these is intended for this kind of use, but in theory we could run \Drupal::service('farm.update')->rebuild(); during one of them.

This would require some more testing IMO... and if we keep hook_rebuild() then it means we'd end up firing that code twice in the cases where there ARE update hooks to run (because the cache would also be cleared).

[paul121]

Looks good! I haven't ran this code locally but the tests make sense. Just one question re: using hook_rebuild() but not a big deal.

At first I thought the FarmUpdate->configUpdate service could be named better but now I see that the ConfigReverter class is registered as the config_update.config_update service - a little odd, but out of our control!

[mstenta]

I just found two hooks that DO fire with update.php

We can consider this in a follow-up, I think. I'm not thrilled about using those hooks for something they aren't intended for.

For now, I just added a 4th step to the update docs that describes how to clear caches as the final step to updating farmOS (only if there are no updates performed during update.php).

[paul121]

Just had a thought - could we print a message when farm.update rebuilds a config? Both to the CLI from drush cr as well as a watchdog message?

Or maybe config_update already does this and we inherit the behavior?

[mstenta]

Ah! Merged before I saw your comment @paul121 . That would be a great follow-up!

I'm not sure if it will be tricky to make it work both ways within the same class, however - as Drush commands use the dt() method for translated logging, while watchdog uses t(). Worth exploring though!

[paul121]

Just opened #447 - it seems that \Drupal::logger('farm_update')->notice(t('My translated notice')) is sufficient for logging and printing translated messages to the console