Problem/Motivation

When the automatic entity schema updates are unable to be processed, exceptions such as

The SQL storage cannot change the schema for an existing field with data.

are thrown. It is not immediately obvious which entity type, and field, the message apples to.

Proposed resolution

Add entity type, and where applicable, field name to the exception.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#5 entity-storage-dx-2542422-05.patch2.99 KBjhedstrom
#2 entity-storage-dx-2542422-02.patch3 KBjhedstrom
#2 interdiff.txt3.59 KBjhedstrom
#1 entity-storage-dx-2542422-01.patch3.25 KBjhedstrom

Comments

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Active » Needs review
Related issues: +#2346013: Improve DX of manually applying entity/field storage definition updates
StatusFileSize
new entity-storage-dx-2542422-01.patch3.25 KB

Something like this provides more details for the developer to start investigating.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
StatusFileSize
new interdiff.txt3.59 KB
new entity-storage-dx-2542422-02.patch3 KB

Actually, just displaying the machine names avoids having to run checkPlain.

The last submitted patch, 1: entity-storage-dx-2542422-01.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented

I really like everything which improves the information behind exception messages. I'm wondering whether we really need sprintf and instead could just use ordinary old string concat, which are IMHO better to read core.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
StatusFileSize
new entity-storage-dx-2542422-05.patch2.99 KB

This removes the sprintf calls.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Yes please!

Was just sitting there yesterday with some of those exceptions and no clue about which field it actually was. We apparently don't have test coverage for those exact exceptions, so I don't think we need to add it here, that might not be trivial.

alexpott’s picture

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

This will makes people's lives easier. Committed 2c15ad9 and pushed to 8.0.x. Thanks!

  • alexpott committed 2c15ad9 on 8.0.x 
    Issue #2542422 by jhedstrom: Improve DX of exceptions thrown in...

Status: Fixed » Closed (fixed)

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