Problem/Motivation

In https://git.drupalcode.org/project/distributions_recipes/-/merge_request..., Wim asked for us to validate that the config actions listed in the config.actions section of recipe.yml all exist.

This is problematic because the actions might not exist until all the modules in the recipe's stack -- that is, everything in the recipe's install list, as well as the install lists of every recipe it implicitly depends on -- are enabled. There is currently no way to "defer" validation to that point. And we don't need to do that -- after all, if a config action doesn't exist, the config action manager will throw an exception, and the recipe should be rolled back. So we should at least ensure we add test coverage.

Issue fork distributions_recipes-3423523

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

phenaproxima created an issue. See original summary.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Active » Postponed
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Title: [PP-2] Validate that every config action listed in recipe.yml actually exists » [PP-1] Validate that every config action listed in recipe.yml actually exists
Related issues: +#3400672: Robustly validate the structure of recipe.yml
narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Title: [PP-1] Validate that every config action listed in recipe.yml actually exists » Validate that every config action listed in recipe.yml actually exists
Status: Postponed » Active
Related issues: +#3424603: Validate that extension listed in the config.import section of recipe.yml has option to bypass all config of that extension
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +DX (Developer Experience)

if a config action doesn't exist, the config action manager will throw an exception, and the recipe should be rolled back. Either way, we should have test coverage for that.

Let's start with adding test coverage for this, so we can ensure the DX is informative, and actually points the Recipe author in the right direction? 😊

narendraR opened merge request !85

narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Status: Active » Needs review

While working on this issue it seems that the failing test is already handling the PluginNotFoundException.
https://git.drupalcode.org/issue/distributions_recipes-3423523/-/jobs/94...

Not sure if the changes I did are required or not.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

You know, that's a good point...what if we changed this line in RecipeCommand:

    catch (InvalidConfigException $e) {
      $this->rollBackToCheckpoint($backup_checkpoint);
      throw $e;
    }

to this:

    catch (\Throwable $e) {
      $this->rollBackToCheckpoint($backup_checkpoint);
      throw $e;
    }

...and just let the plugin system do its thing? Rather than trying to introduce some sort of complicated validation? We could keep the expanded test coverage.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Title: Validate that every config action listed in recipe.yml actually exists » Recipe runner should fail and roll back if a recipe tries to use a non-existent config action
Status: Needs review » Needs work
Issue tags: +Needs issue summary update

No complaints here; just retitling for clarity on the actual resolution.

Does need an issue summary update, though.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Title: Recipe runner should fail and roll back if a recipe tries to use a non-existent config action » Test that recipe runner fails and rolls back if a recipe tries to use a non-existent config action
Category: Feature request » Task
Issue summary: View changes
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs issue summary update
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed f4b42298b1e to 11.x and 87ae57111d8 to 10.3.x. Thanks!

  • alexpott committed 87ae5711 on 10.3.x 
    Issue #3423523 by phenaproxima, narendraR, Wim Leers: Test that recipe...

  • alexpott committed f4b42298 on 11.x 
    Issue #3423523 by phenaproxima, narendraR, Wim Leers: Test that recipe...

  • d806e137 committed on patch 
    Update recipe 10.3.x patch 87ae5711 Issue #3423523 by phenaproxima,...

  • 840fefb6 committed on patch 
    Update recipe 11.x patch f4b42298 Issue #3423523 by phenaproxima,...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.