🇦🇺Australia @kim.pepper

Fixed the failing test by handling the exception. Not sure if this is the right place to handle it.

Added 📌 Move file form hooks to FileFormHooks class Active for the hooks cleanup mentioned above.

Added a todo for replacing the static \Drupal\file\Hook\FileFormHooks::settingsSubmit() with a non-static once 📌 Use CallableResolver for form callbacks Active is in.

kim.pepper → created an issue.

Yes. I discussed this with @fago in slack and we decided to move it here. We can re-use the connection plugin logic and existing search api implementation.

kim.pepper → made their first commit to this issue’s fork.

Yes CallableResolver supports the service notation.

kim.pepper → created an issue.

We need a IS and title update

Looks like this should be re-opened based on the comments above.

Awesome thanks! When can we expect this in a release?

We should actually do this in search_api_opensearch. Created a new issue 📌 Add support for AI module VDB Provider Active

Closing as a duplicate of 🐛 Check for new files when validating size Needs work

@mutasim al-shoura You had commented on that issue already. Not sure why you opened a new issue?

Agree with @berdir. Using CallableResolver gives us more options and proper validation of callables.

kim.pepper → created an issue.

kim.pepper → created an issue.

kim.pepper → created an issue.

kim.pepper → created an issue.

This caused a BC break in 🐛 Make compatible with 1.1.x series of the AI module Active What's the BC policy for this module? I assumed we can't just add params to interfaces in minor releases?

Looks like a BC breaking Drupal\search_api\Query\QueryInterface param was added to \Drupal\ai\AiVdbProviderInterface::vectorSearch in ✨ Pass metric type to vector search function Active .

We don't seem to need it?

I think it could provide some nice interfaces and models for us to use.

kim.pepper → created an issue. See original summary → .

kim.pepper → made their first commit to this issue’s fork.

As suggested by @catch I've split off adding the composer dependency in ✨ Add PSR-20 Clock Interface as composer dependency Active and postponed this issue on that. Moved the dependency evaluation details to the other issue.

As suggested by @catch I've split off adding the composer dependency to reduce the rebase burden.

kim.pepper → created an issue.

Chatting with @mohit_aghera in slack and we have a new approach based on a semantic field type in OpenSearch 3.1. This greatly simplifies the amount of work needed to support neural search.

There is a bug with remote text embedding models, which was recently fixed and merged but not yet in a release. https://github.com/opensearch-project/neural-search/issues/1426

kim.pepper → created an issue.

If we pass the index then we no longer need to pass the backend config as we can get it from the index.

We'll need to manage BC for that.

Applied suggestion and self-RTBC as per #10

I changed this to a static method, but seems strange to me that a submit handler must be static.

How does this work for Hooks that use dependency injection and autowiring?

Interesting. I'm not sure how autowired classes will work when using a classname. I was sure they were for static methods, but lets see what the tests say.

I feel like we need more support for this issue, otherwise we'll be endlessly rebasing it.

Reviewed this again. I think it is good to go, but will leave as NR for others.

kim.pepper → created an issue.

kim.pepper → created an issue.

So it's just adding `requiredKey: false` under the playsinline block in the schema?

Rebased on 11.x and moved everything over from system_requirements() to \Drupal\system\Install\Requirements\SystemRequirements::checkRequirements()

Working on this

Hmm. If it's not a hook then I assume it's not getting called. We already have a post-update hook for setting default values, so I guess it's dead code. Created a follow up 🐛 Remove unused file_file_video_presave() Active to remove.

kim.pepper → created an issue.

Fixed conflict with 📌 [pp-3] Split up and refactor system_requirements() into OOP hooks Active and added check for setting in SystemRequirements::checkRequirements() instead of system_requirements().

Added a check for the setting in system_requirements().

I will leave for 24hrs to see if @jcontreras wants to fix #16.

Sounds like a great idea.

Changing priority to Major as per guidelines https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... →

It was a bot triggered comment that sent an email. I just rebased it.

xjm → credited kim.pepper → .

I think we need an answer to #3

Re: #23 I updated \Drupal\file\Entity\File::getMimeType() to return the default mime type if NULL, and also the docs on \Drupal\file\FileInterface::getMimeType() to match this behaviour.

Good catch. If we used \Drupal\file\Upload\FileUploadHandler::class as the service name this would probably have been caught sooner.

Looks like it's a random fail 🐛 [random test failure] ScaffoldUpgradeTest and ScaffoldTest rely on packagist.org Active

Looks like the test fail is an unrelated PHP 8.5 test.

Rebased on 11.x and fixed tests. We had space ' ' in the list of special chars, but this is handled separately.

kim.pepper → created an issue.

Ran into a bit of an issue with the pipelines. In order to have Opensearch generate the text embeddings, you need to specify text field to embedding field mappings when creating the pipeline. I don't think it would be easy to dynamically create a pipeline like this with search api.

I'm going to check out the https://www.drupal.org/project/ai_vdb_provider_opensearch → module to see if the built-in AI Search would work.

Started work on a more integrated approach. At this stage all the MR does is set index.knn = TRUE when creating the index.

In order to have knn enabled on an index, we need to set that option when creating the index. We can change it after.

This meant we needed to refactor the addIndex() method to not create then update settings, but to pass the settings at creation time. This refactoring could potentially be split out into a separate issue.

I reviewed the changes and confirmed that all use of FileSystemInterface::basename() have been converted to basename().

My only question is on which version this will be removed in.

@deprecated in drupal:11.3.0 and is removed from drupal:13.0.0.

Is it too late to have it removed in drupal 12.0.0 ?

Looks like you are correct. I'd be happy to deprecate/remove.

Confirmed that:

• the bug linked in the comment is no longer relevant.
• Directory separators are handled natively
• The behavior of php basename() seems to handle directories ending in slash just fine.

   * PHP's basename() does not properly support streams or filenames beginning
   * with a non-US-ASCII character.

@see http://bugs.php.net/bug.php?id=37738

Here's an 3v4l snippet to test the standard basename() behavior:

https://3v4l.org/5oTmX

Committed to 3.x and 2.x. Thanks!

Updated to catch both OpenSearchExceptionInterface and ClientExceptionInterface

Hmm. What version of opensearch-php are you using?

We should be able to just catch OpenSearchExceptionInterface as we convert any HTTP responses with status >= 400 to an instance of \OpenSearch\Exception\HttpExceptionInterface which extends \OpenSearch\Exception\OpenSearchExceptionInterface.

Thanks for reporting. We actually need to remove \OpenSearch\Common\Exceptions\OpenSearchException as it is deprecated in opensearch-project/opensearch-php 2.4.0

Mohit and I discussed this in Slack and he agreed for me to post this request.

kim.pepper → created an issue.

xjm → credited kim.pepper → .

A recommended approach for vector indexing is an ingest pipeline. I wonder if this issue could be expanded to include support for that?

Create an MR against 3.x and it will get cherry-picked into the 2.x branch.

Thanks for reporting this issue. Can you please create a MR for it so automated test pipelines will run?

I reviewed this issue as part of Bug Smash Initiative.

I did not try to reproduce this locally. It looks like the code is almost identical to when the issue was posted.

Unfortunately there is no context to that change to inform whether it is intended behavior for security reasons.

It would be helpful to have an automated test to prove the bug exists.

I'm changing to PMNI to get feedback from the OP and see if it is still occurring.

griffynh → credited kim.pepper → .

Hiding files

smustgrave → credited kim.pepper → .

This should only go in 2.x as we want to remove it in 3.x

I created a follow-up issue to restore the orginal behavior. 🐛 Restore querytime_synonyms analyzer Active

kim.pepper → created an issue.

It's been 3 years. Any appetite for a 2.x branch to begin removing deprecated code and adding typehints?

Committed to 2.x and 3.x. Thanks!

kim.pepper → created an issue.