Problem/Motivation

#2733331: Store as sequence introduced a post update hook for changing the configuration to a sequence but if the hook is run after the configuration has already been updated (by a config sync operation, for example) the execution will fail and remain stuck.

Example error message from database update script

InvalidArgumentException: The configuration property third_party_settings.allowed_formats.allowed_formats.0.0 doesn't exist. in Drupal\Core\Config\Schema\ArrayElement->get() (line 76 of /app/docroot/core/lib/Drupal/Core/Config/Schema/ArrayElement.php).

Proposed resolution

Update the post update hook to take no action on configuration that has already been updated (patch incoming).

CommentFileSizeAuthor
#2 3261891-2.patch877 byteswells

Issue fork allowed_formats-3261891

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

wells created an issue. See original summary.

wells’s picture

wells
he/him/his
Primary language English
Location Seattle, WA
commented
Status: Active » Needs review
StatusFileSize
new 3261891-2.patch877 bytes

Patch attached based on proposed resolution.

yuseferi’s picture

yuseferi commented

Nice!
worked for me

jklmnop’s picture

jklmnop commented

#2 worked for me too.

kjwhite’s picture

kjwhite commented
Status: Needs review » Reviewed & tested by the community

Tested patch in #2 successfully.

mvogel’s picture

mvogel commented

+1

duaelfr’s picture

duaelfr
he/him
Primary language French
Location Montpellier, France
commented

Nice patch! Thanks :)

abu-zakham’s picture

abu-zakham commented

+1

artusamak’s picture

artusamak
he/him
Primary language French
Location Bzh
commented

I can also confirm that #2 is fixing the issue. Thank you for the patch.

Sashaz made their first commit to this issue’s fork.

dww’s picture

dww
we/he/they
commented

Another +1 for the RTBC. Hit exactly this problem and patch #2 solves it. I can't find anything to complain about for the patch: comment is clear, code looks good. Confirmed it solves the problem. I don't know that we need an automated test for this, I think we should just fix it.

If I were a maintainer here, I'd remove credit from @Sashaz for clicking the link to create an issue fork when there's already a patch and doing nothing else with it. Creating an MR would be pointless for such a small patch. IMHO, this looks like someone trying to game the credit system without doing any meaningful contribution...

Anyway, thanks @wells for opening the issue and the nice patch, and everyone else who actually tested this to confirm it's working!

Cheers,
-Derek

  • wells authored b906529 on 8.x-1.x 
    Issue #3261891 by wells: Post update hook fails when config is already...
nord102’s picture

nord102
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Thanks for catching this and for the patch @wells. Thanks everyone else for all of the feedback.

Status: Fixed » Closed (fixed)

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

eelkeblok’s picture

eelkeblok
he/him
Primary language Dutch
Location Netherlands 🇳🇱
commented

For those stumbling upon this after the fact, one puzzling aspect of this is that our deployments always do updates *first* and only then do a config import, so this really shouldn't happen. It did anyway.