CMI 2 meeting - 2020-03-18 [#3120594]

Meeting will happen in #config on drupal.slack.com.

---

Hello and welcome to this CMI 2.0 meeting!

This meeting:
➤ Is for core and contributed project developers as well as people who have an interest in advancing the configuration management capabilities of Drupal.
➤ Usually happens every other Wednesday at 16:00 UTC.
➤ Is done over chat.
➤ Happens in threads, which you can follow to be notified of new replies even if you don’t comment in the thread. You may also join the meeting later and participate asynchronously!
➤ Has a public agenda anyone can add to: `https://www.drupal.org/project/drupal/issues/3120594`
➤*Transcript will be exported and posted* to the agenda issue. For anonymous comments, start with a :bust_in_silhouette: emoji. To take a comment or thread off the record, start with a :no_entry_sign: emoji.

---

Ping @bircher @alexpott @diosbelmezquia @mtodor @ricardoamaro @borisson @jcandan @marcvangend @Kingdutch @phenaproxima @balsama @wouter @mikemadison (Edit https://www.drupal.org/project/drupal/issues/3120595 if you do or don't want to be pinged)

---

0️⃣ Who is here today? Comment in the thread below to introduce yourself.

kingdutch	I’m busy with some other things, mention me if you need me :smile:
bircher	Yea crazy times! I am pretty busy too, but we postponed the last meeting already so its good to have one at last.
bircher	it is on slack after all and will be available also tomorrow
jungle	Jungle, Chongqing, China
bircher	Hi @jungle! welcome to the CMI 2 meeting!
jungle	Thank you, @bircher, Just found the meeting is happening, and follow :slightly_smiling_face:

1️⃣ Do you have suggested topics you are looking to discuss? Post in this thread and we'll open threads for them as appropriate.

jungle	Just want to mention this issue #3112252: Change the filesystem structure of config YAML files
bircher	@jungle Thanks for the input, but your proposal has a glaring flaw: directories in the sync storage already have a significance, in particular they are used for what is called collections. Collections is how for example the language system keeps translations of the config. But there may be other collections too.
jungle	Thank you! Sorry. I can’t understand the above all. maybe i should do a POC with the given comments first.
bircher	sure! just make your POC with the umami demo profile, it is multilingual and then you will see what I mean
jungle	Got it. Thanks for your direction!
richardbporter	Probably too late to jump in here but I was looking for feedback and/or direction on #3117841: Conditionally-split configuration removed from sync directory.
bircher	@richardbporter not too late :slightly_smiling_face: I was thinking about a way to address this issue, and I have some idea, I posted on the issue.
bircher	initially I din't think it would be that simple and that we should check dependencies... but I guess if you graylist it too you know what you are doing and we don't need any hand holding for potential missed dependencies
richardbporter	Thanks. I’ll take a look.

2️⃣ Installing from config and install hooks of profiles

bircher

This is the most actionable issue for this: https://www.drupal.org/node/2982052

3️⃣ Other contrib updates to share

bircher	I made some proof of concept module to address [#2901418]it can be debated on how it should be called or if it is needed at all and what the drush commands should be. But here it is https://github.com/bircher/drupal-deploy_hook/
bircher	currently most people use the sequence that @clau posted in comment 66 -#2901418: Add hook_post_config_import_NAME after config import#comment-13312081 (edited)
bircher	iedrush updb --no-post-updatesdrush config:importdrush updb
bircher	but I think it is better when post_update hooks run after the update hooks and not in a significantly different site (which it may be after a config import)and also splitting updtae and post_update hooks can not be done from the UI. I could imagine adding a form to my poc module to run the "deploy" update hooks with a batch. But I don't know if that is a real concern
moshe	I would love input from @alexpott or @catch about the intent of post_update hook. Is it OK that it runs well after hook_update_n, like in that clau example. If not, lets use your contrib module. I’m not familiar enough with the design/intent to know how best to proceed.
catch	It should run before config imports.
catch	Because it is often updating config.
catch	Then export the configuration after post updates have run, and the config import then has less to do.
moshe	A deployment rarely runs config:export. That would overwrite the incoming config that was carefully approved via a QA process, etc.
bircher	in development I guess you run both update and post update hooks together then you export config, then on deployment the config import and the post update hook change it to the same value, probably.
catch	Yes it should be like this:Locally:drush updbdrush config:export + commitDeploy:drush updbdrush cim
catch	The drush cim then might be a no-op, or you might have actual config changes for other reasons. (edited)
catch	but drush cim should not be responsible for doing what the post update did on your development environment.
moshe	Oh yeah that makes sense. Would be good to know if that needs the needs for @clau and @pfrenssen.
clau	@catch @moshe Well, cim might create new fields. In some cases you’ll need to perform content operations with the new fields after cim. E.g. fill the new field with values. I don’t know why post update would update config. I know it currently happens but I think it shouldn’t. IMO, hook_update_N() and cim, both are meant to repair the API. Adding/removing a field via cim is also API fixing. Post updates are meant to run on a healed API and make use of the new API. How could a post update run with the new API if cim, didn’t enabled yet the new modules or added the new entity types, new bundles, new fields? (edited)
bircher	@clau in my understanding in update hooks you have to assume that only the bare minimum works, so you can get data out of the database if needed and you can do the schema updates. Then post_update hooks run just after that where drupal services are back to normal. So if you are a core or contrib module those two hooks is what you have, so it makes sense to me that you change configuration and add the data back in the post_update hook. Config import (and export) are a deployment mechanism by which you update the configuration like new fields and new bundles etc that were added via site-building. If you write code that expects the new fields to be there you have to create the new field in a post_update hook.I think there are two things that are confused, for a module author config import and export are not available as update mechanism. When you develop on a site then you do have config import and export and yes there is the need as part of the deployment to run some things after the config import step of the deployment. That is why I created the issue with the post_config_import hook, but I now think it is better not to tie things together too much and hence, I created the new module that runs update-hook-like hooks with a separate drush command. ie post_update hooks are for module developers deploy_hook hooks are for site builders who need to run things after the config import (you can still use post_update hooks that run before the config import on a deployment)But both approaches work in most cases, so maybe it is a matter of preference?
alexpott	@clau I disagree that cim is for API fixing. If you have code that depends on a field the there should be either a post update or a hook_update_N that adds the field
clau	@alexpott enabling a module, isn’t that new API?
alexpott	New is different from fixing. If you have code that you changed to depend on a new module then you must write a hook_update_N to enable that module.
clau	@alexpott Well, correct. But I want to update some content with the new module, when deploying
clau	then you must write a hook_update_N to enable that module.
alexpott	You need to decouple that.
clau	That is very bad. We had this for years and I can tell that is gives a lot of errors.
alexpott	Because you are trying to do too much a once.
alexpott	Well how drush 10 cim is broken is very bad.
clau	I know, but that is other topic
alexpott	cim should not ever be done using the UpdateKernel it’s a completely untested and unsupported.
alexpott	But they reason you implemented is to support doing cim inbetween hoo_update_N and post updates.
alexpott	If your code dependencies change and those include dependencies on content / config then you need to use the update system to manage that. Using the config system is unsupported because we have no guarantees about the state of the system.
clau	No, I don’t think. I don’t remember the reason.
alexpott	Both #2762235: Ensure that ConfigImport is taking place without outstanding updates are relevant here.
clau	I know, the tickets. If we’re gonna make cim run only after post-updates, than we need a new post-cim-phase/hook. (edited)
alexpott	There’s an issue for that :slightly_smiling_face:
alexpott	But also I suspect that that issue only really exists because people are mixing config and code updates.
clau	Then, I’m not really sure anymore that I can distinguish between the scope of updated and post-updates (edited)
alexpott	But I do think that the ability to perform automated content updates post a config import is real. And should be supported
clau	Let’s take a simple example: I’m migrating data from a field to other new field and remove the old field. How should this be handled? (edited)
clau	All process in hook_update_N()?
alexpott	I would handle that in a post update hook. That would do everything. From creating the old field to importing the new field.
clau	As DX, I prefer to let cim create the field. Then, with hook_running_after_cim(),, I’m only migrating data and remove the old field
alexpott	You are free to read from your config sync directory in your post_update to get the field config to create.
alexpott	And there are issues out there to make that easier. And we should probably explore them.
clau	The other issue with UpdateKernel I should ask @pfrenssen. I don’t recall why that was needed.
alexpott	According to the github issue it is because you were trying to enable a module which other modules depend on because you have code changes to them too.
clau	True. But then, how should this be fixed? Is there a d.o issue?
alexpott	Many :slightly_smiling_face:
alexpott	#3002026: Improve API for creating/updating configuration come to mind
alexpott	The list on the project page of https://www.drupal.org/project/config_update is good too
bircher	clau As DX, I prefer to let cim create the field. Then, with hook_running_after_cim(),, I’m only migrating data and remove the old field.yes, and that was the reason I made https://github.com/bircher/drupal-deploy_hook/ which started this discussion. I am no longer a fan of the post_config_import hook because config import (and export) is part of developing and I don't want this to interfere. So I think it is better if it is a separate step and not everything happens all at once. if you want to codify running several things at once we use https://github.com/openeuropa/task-runner (but there are other solutions like makefiles) which invoke all the drush commands with the correct arguments in the correct order.
moshe	Given that we have heard same from @alexpott and @catch, I think we should:Revert UpdateKernel PR from Drush.@clau, myself, and everyone else should change our deploy process as below.(edited)
moshe	OLDdrush updb --no-post-updatesdrush config:importdrush updbNEWdrush updbdrush cimdrush deploy-hook:runThat last command comes from https://github.com/bircher/drupal-deploy_hook/ (edited)
bircher	@moshe ok then I'll make that into a real drupal module...
bircher	anyone wants to be co-maintainer? suggestions for module name?
alexpott	well “deploy” in Drupal terms is taken. And config_ is way over used…
alexpott	Naming it after the cli command gets a :+1: from me
bircher	well the cli command is named after the module :smile:
alexpott	That said I wonder if we should make it add the same thing we’re adding in #2901418: Add hook_post_config_import_NAME after config import so when that does land in core we can deprecate the module.
moshe	Would it make sense for it not to be a module but instead put the whole thing in core Drush? If it makes sense and @bircher is up for maintaining (and another?), I’m OK with it.
bircher	@moshe it can also be in core drush, no problem with that!
bircher	@alexpott so you would you recommend having a hook that runs after every config import rather than a new step?
alexpott	I dunno
alexpott	A new step would need a new runner in core. So once advantage of piling on to the end of config is we have runner already
alexpott	A new step has the advantage of not being coupled to config - which imo is a good thing.
bircher	yes the hook_post_config_import_NAME is also a module on my github from three years ago, and I have come to change my mind and prefer a decoupled hook.
alexpott	+1 to experience
bircher	if it is a module then we can add a UI to it, and experiment to see if we want to add it to core
moshe	It would be a big task to add a “Perform deployment” to the core UI
bircher	ie after a completed config import you get redirected to the page with the pending post_config_import hooks and you can press "run" and have another batch running
moshe	I personally am not interested in working on that.
alexpott	Yeah me neither really. I think if you need this you’d like to use the CLI for it.
bircher	me neither :smile:
bircher	yea.. ok cli only.. drush core is the best place! and we close the core issue..
alexpott	I think we need to define what a deployment is. And build the functionality into drush / or something that can extend drush to deliver that
alexpott	For me it feels like a deployment can contain:run hook_update_Nrun post updatesrun config importRun other stuff
alexpott	This thing should help you do that in the right order. So for example I don’t think hook_update_N and post updates should be decouplable via the drush runner (as they currently are)
alexpott	I also think we should discourage running commands prior to running hook_update_N.
alexpott	What happens post hook_update_N & post updates feels more fluid.
moshe	Yeah, clearly Drupal ecosystem needs a canonical deploy command. I’ve been hesitant to add those to Drush, since they tend to need more maintenance. Notice how quick:drupal was booted from Drush core (and now its mostly in core Drupal). Still, this need seems urgent enough I’ll consider it.
moshe	@clau Any chance you can prepare a rollback PR for Drush?@bircher if possible please submit a Drush PR for the new deploy hook. the hook name isn’t super important since most people will only run it via a new deploy command.(edited)
bircher	@moshe :thumbsup: yes I'll create a PR for it
moshe	I made a deploy command for core Drush. It depends on @bircher PR. https://github.com/drush-ops/drush/pull/4359. Lets discuss in that PR or in #drush