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

CommentFileSizeAuthor
#6 requirements-errors.png36.14 KBscott_euser

Issue fork ai-3483356

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:

Comments

marcus_johansson created an issue. See original summary.

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

scott_euser’s picture

Status: Active » Needs review

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
  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.

scott_euser’s picture

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"
scott_euser’s picture

StatusFileSize
new36.14 KB

Added back in info.yml
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.

Updates from AI alpha 8 to further jump when deprecated modules completely removed
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...

Hook requirements additional helper
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.

Screenshot of updb requirements error

marcus_johansson’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

kristen pol’s picture

Component: Providers » AI Core module
Assigned: marcus_johansson » Unassigned

We are doing some issue management housekeeping and adding/removing components.

The "Providers" component will be removed because providers have been moved into their own projects, so this issue is being moved to "AI Core module".

See #3533272: Update AI module project components for more details.