- 🇩🇪Germany a.dmitriiev
Re-rolling the patch from #8, still on top of 8.x-7.x branch, as it was found out, that
nodespark/des-connector
in version 7.x also usesruflin/Elastica
and it is also seen that some development is still happening in this branch.Re-roll has one extra change: new configuration setting for the datasource
combined_id
. It was added in case you want to read the documents from the index that was created also but Search API from different Drupal website and the id already has the Drupal datasource like `entity:node` or something, then SearchFactory will not add the elasticsearch document datasource prefix, that is actually needed to recognize that the document is an external entity. - 🇩🇪Germany a.dmitriiev
Added type hint to
\Drupal\elasticsearch_connector\Plugin\DataType\ElasticSearchDocument::getIterator()
to be compatible with\IteratorAggregate
- 🇩🇪Germany a.dmitriiev
I found out, that the deriver for the datasource should have also cluster id in the plugin id, because in case there are more than 1 cluster and the index names on some are the same, there will be a collision
- 🇩🇪Germany a.dmitriiev
I found a problem in loadMultiple function of ElasticSeachDocument, in case the number of ids to load is bigger than default items per page (in my case it was 10), then only 10 items were loaded out of all. This is causing problems for example in views, where the ids are passed to this method and expect all items to be loaded
- 🇩🇪Germany a.dmitriiev
Fixing the problem with SearchBuilder line 204, as it should still be
getFieldIdentifier
and notgetPropertyPath
- 🇪🇸Spain manuel.adan 🌌
manuel.adan → changed the visibility of the branch 8.x-7.x to hidden.
- 🇪🇸Spain manuel.adan 🌌
Created new branch 3102388-nodespark_des_connector_data_source in the issue fork for works on the 8.x-7.x (nodespark/des_connector based) branch only with the implementations from #3102388-8 updated to the latest base code changes.
Patch from #20 ✨ Add Search API data source implementation Needs work , which contains several changes from the lastest comments, brokes my site currently running previous patch version, probably because of the plugin ID change. We have no easy way to review changes in patches from #15 ✨ Add Search API data source implementation Needs work to #20 ✨ Add Search API data source implementation Needs work since there is no interdiff and also some patches contain more than one change. Changes from latest patch files should be commited on top of the new branch, as 1 commit per change, in order to be able to follow and review them.
- 🇪🇸Spain manuel.adan 🌌
Patch file attached containing plain diff of current state of branches issue/3102388-nodespark_des_connector_data_source and origin/8.x-7.x for convenience only.
- Merge request !49Issue #3102388: Add Search API data source implementation → (Open) created by a.dmitriiev
- Status changed to Needs review
8 months ago 2:54pm 8 April 2024 - Open on Drupal.org →Core: 10.2.1 + Environment: PHP 8.1 & MySQL 5.7last update
8 months ago Waiting for branch to pass - Open on Drupal.org →Core: 10.2.1 + Environment: PHP 8.1 & MySQL 5.7last update
8 months ago Waiting for branch to pass - Open on Drupal.org →Core: 10.2.1 + Environment: PHP 8.1 & MySQL 5.7last update
8 months ago Waiting for branch to pass - Open on Drupal.org →Core: 10.2.1 + Environment: PHP 8.1 & MySQL 5.7last update
8 months ago Waiting for branch to pass - Open on Drupal.org →Core: 10.2.1 + Environment: PHP 8.1 & MySQL 5.7last update
8 months ago Waiting for branch to pass - 🇩🇪Germany a.dmitriiev
I have applied all the changes from my patches, one by one and created MR https://git.drupalcode.org/project/elasticsearch_connector/-/merge_reque... . Please review
- 🇩🇪Germany a.dmitriiev
I found that sometimes loadMultiple is missing 1 item. This causes search_api to return error message like:
Could not load the following items on index : "elasticsearch_document:external:/entity:node/92610:en".
When the range is extended, this message is not there anymore, and I can confirm in xdebug, that loadMultiple returns all items.
Interdiff is attached, MR is also updated.