Move out all provider modules to contrib modules

Created on 25 October 2024, 29 days ago

Problem/Motivation

Currently we have first class provider citizens being in the core module and second class providers being contributed, that in some cases are more advanced, leads to a disperity in what people knows to be available.

Instead we should just let recipes solve the problems with wanting to pack a specific provider to a specific functionality or let the end user decide what kind of AI that want to solve a specific issue.

We also have the problem that there are operation types that doesn't have a core module that supports them, which might be confusing - if you instead install specific providers for specific solutions it gets easier to understand.

Steps to reproduce

Proposed resolution

1. Move the providers to contributed modules.
2. Pull them in temporarily using composer.json
3. Make the modules empty, deprecated and let them have an update hook that updates the settings, installs the new module, uninstalls itself and clears cache.

We shouldn't currently have any configuration dependencies on the actual module and since the plugin name will be the same this should work.

Remaining tasks

User interface changes

API changes

Data model changes

Feature request
Status

Active

Version

1.0

Component

Providers

Created by

🇩🇪Germany marcus_johansson

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @marcus_johansson
  • First commit to issue fork.
  • Merge request !224Resolve #3483356 "Move to contrib" → (Merged) created by scott_euser
  • 🇬🇧United Kingdom scott_euser

    This works, but:

    1. Needs further testing as we need to make sure we get this right
    2. Needs update hooks in the external modules just like the one added to Pinecone: https://www.drupal.org/project/ai_vdb_provider_pinecone/issues/3483565 📌 Update hook from vdb_provider_pinecone setting to ai_vdb_provider_pinecone Active
    3. Needs to have further deletions of all moved modules
    4. Needs to have further deletions from composer.json
    5. Needs to have further deletions in docs folder

    So sort of needs work/needs review dual state.

    The files to look at to review:

    1. ai.install -> in particular see the Scenarios bit
    2. ai.module -> prevents accidental uninstall; see the comments, the user could really want to uninstall and not switch, so there is a route

    I put detailed comments in both of those there.

    The deletions add noise to the MR, but are needed for tests to pass since composer dependency gets removed (in example of pinecone so far).

    Temporary alternative:
    One alternative has been suggested of setting all submodule new external module versions as composer dependencies. That kicks the can down the road as it doesn't force the switchover, so still update hook is needed so AI core module can enforce stopping usage of the old modules and let us safely delete them from here.

    It also means that sites then get a lot more top-level modules temporarily. Same amount of code as is of course, but some users may be unhappy with that.

    So I think we should aim to try to get it working well with a full removal.

    What I tried
    I tried a couple runs at this with pinecone to test:

    1. Enabled external pinecone module -> old one successfully uninstalled and config copied over: this is the simplest case
    2. Ran updb without any knowledge of new external module -> ai core told me to composer require the external pinecone module.
    3. Run updb with composer require of new external module -> ai core module triggered the install file of pinecone which then did (1)

    Side note: I checked with Marcus in slack first before working on branch, to check he hadn't started since its assigned to him.

  • 🇬🇧United Kingdom scott_euser

    Not 100% sure if we need to leave placeholder info yml files in place for a bit...
    e.g.

    lifecycle: deprecated
    lifecycle_link: "https://www.drupal.org/project/ai/issues/3483356"
    
  • 🇬🇧United Kingdom scott_euser

    Yep, its not possible to run e.g. `drush cr` without it because of the module name change. I think this must be why Drupal Core forces you update to the latest within Major version. I added back in the info.yml file. There is still a risk regardless of scenario that a user is on e.g. alpha8 or earlier, and doesn't update e.g. for a few modules to some point when we have removed the deprecated submodules though. In that case, we could either decide:

    1. Its alpha, its okay - the edge case will come to issue queue and find similar issue
    2. We increase semver to 2x, so user's will have to more consciously do it

    (2) is probably the proper way and matches Core movement of modules from Core to contrib I think, but not a hill I'll die over...

    I additionally added this morning hook_requirements as well, which happens even earlier than updb. Still the user can skip this, so I left that particular scenario in the updb hook.

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

Production build 0.71.5 2024