cakephp / elastic-search

Elastic search datasource for CakePHP
Other
88 stars 53 forks source link

'Refresh' behavior #275

Closed andrii-pukhalevych closed 1 year ago

andrii-pukhalevych commented 2 years ago

Currently, 'refresh'=>true option for save() and saveMany() causing additional elastic query $esIndex->refresh().

I see that it is possible to pass refresh parameter in primary request: https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-bulk.html#bulk-refresh https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-update.html#docs-update-api-query-params

Should not we remove $esIndex->refresh() in favor of passing 'refresh'=>true in primary request?

andrii-pukhalevych commented 2 years ago
public function saveMany($entities, $options = [])
....
    $requestParams = [];
    if ($options['refresh']) {
        $requestParams['refresh'] = 'wait_for';
    }

    $esIndex = $this->getConnection()->getIndex($this->getName());
    $esIndex->addDocuments($documents, $requestParams);

And remove below:

if ($options['refresh']) {
    $esIndex->refresh();
}
markstory commented 2 years ago

Makes sense to me. I don't think the refresh option existed when saving was first implemented, but now that it exists we should use it.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open for 120 days with no activity. Remove the stale label or comment or this will be closed in 15 days