Problem/Motivation
Recipes are supposed to be idempotent, safe to apply twice or more 🤷‍♂️. That's the whole reason core actions like entity_create:createIfNotExists exist. This recipe isn't idempotent. Applying it a second time on a site that already has it fails outright.
The direct cause: setupVdbServerWithDefaults (on search_api.server.content_vector) and setupVdbIndex (on search_api.index.content_vector) both call $this->entityTypeManager->getStorage(...)->create($value)->save() unconditionally, with no check for whether the entity already exists. That code lives in the ai module, not in this recipe, so the actual fix might not be something this recipe's maintainers can make on their own.
The real question to settle first is whether idempotency is a goal for this recipe at all. If it is, that's a constraint on which actions it can use, and probably a conversation with the ai module's maintainers about adding an existence check there. If the answer is no, that's a fair call too, but it should be made on purpose, not left as a side effect of which action happened to get picked.
Steps to reproduce
- On a fresh site:
drush recipe:apply ai_recipe_content_search_vector. - Apply it again:
drush recipe:apply ai_recipe_content_search_vector.
Second apply fails:
In SetupVdbServer.php line 116: Could not save the configuration.
Proposed resolution
Confirm idempotency is actually wanted here. If it is, raise an existence check with the ai module's maintainers for setupVdbServerWithDefaults and setupVdbIndex, matching what entity_create:createIfNotExists already does. Happy to submit an MR once that's agreed.
Remaining tasks
- Decide whether this recipe should be idempotent.
- If yes, coordinate with the
aimodule's maintainers on adding an existence check toSetupVdbServer::apply()andSetupVdbIndex::apply().
Issue fork ai_recipe_content_search_vector-3620136
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:
Comments
Comment #2
abhisekmazumdarOkay, this also means that when https://www.drupal.org/project/ai_recipe_answers marks this recipe as a dependency,
ai_recipe_content_search_vectorwill be applied again, which will break the setup. It would be great if we could makeai_recipe_content_search_vectoridempotent.Comment #3
a.dmitriiev commentedI also believe this should be fixed in AI Core. But be aware, that as recipe passes some server and index values, probably when checking whether config entities already exist, you also need to check that the provided values from the recipe match the existing values, as normally the AI Server settings can't be changed (embeddings model, size of chunks, vector dimensions) as this will require wiping out the remote index.
Comment #4
abhisekmazumdarComment #5
arianraeesi commentedComment #6
abhisekmazumdarFiled a companion issue against the
aimodule: 3586729. That's where the actual fix needs to happen: an existence check inSetupVdbServer::apply()andSetupVdbIndex::apply(), matching what core'sentity_create:createIfNotExistsdoes.So I'm marking it postponed until that happens...
Comment #7
a.dmitriiev commentedThe issue in AI Core was merged and will be added to the next 1.5.x release.
Comment #8
abhisekmazumdarI verified that the fix from https://git.drupalcode.org/project/ai/-/work_items/3586729 resolves this.
I tested it, and it works. This recipe can be applied twice, which makes it usable as a dependency in other recipes too. That is especially useful for https://www.drupal.org/project/ai_recipe_answers.
The existence checks in
SetupVdbServer::apply()andSetupVdbIndex::apply()work. Reapplying this recipe is now safe.One thing worth deciding
This recipe's
composer.jsoncurrently requiresdrupal/ai: ^1.3, so a fresh install can still pull in 1.3.x or 1.4.x, and neither includes the idempotency fix. Anyone installing this recipe on one of those versions will hit the same bug.I would bump the constraint to
drupal/ai: ^1.5once that release ships.Comment #10
abhisekmazumdarComment #11
a.dmitriiev commentedLet's pin the version to 1.5.0@RC as this version https://www.drupal.org/project/ai/releases/1.5.0-rc4 already has it included
Comment #13
abhisekmazumdarDone MR opned.
Comment #14
a.dmitriiev commentedPlease check my comment in the MR
Comment #15
abhisekmazumdarDone back to review.
Comment #17
a.dmitriiev commentedMerged!