Allow to pass native endpoint options to index creation #1859
Conversation
deguif
force-pushed
the
create-index-options
branch
2 times, most recently
from
ed93a8d to
2df1cd5
Compare
| @@ -422,7 +423,9 @@ public function create(array $args = [], $options = null): Response | |||
| } | |||
| } | |||
|
|
|||
| $endpoint = new Create(); | |||
| unset($options['recreate']); | |||
There was a problem hiding this comment.
Can you share why this is needed now?
There was a problem hiding this comment.
Yes, I had in mind to use option wait_for_active_shards when creating indexes on ES.
This would remove the need to have the function Elastica\Test\Base::_waitForAllocation() for example by simply passing this option when creating the index.
There was a problem hiding this comment.
I think I need to read up on my end what this flag does or doesn't.
Is it now with the new code that recreate or was this always the case and now we remove it but didn't before? Trying to understand which part is related to the change of getting the options and what is further improvements.
There was a problem hiding this comment.
The unset is here only to remove invalid endpoint options so that these options can be passed to the endpoint via setParams().
| @@ -402,9 +402,10 @@ public function create(array $args = [], $options = null): Response | |||
| throw new \TypeError(\sprintf('Argument 2 passed to "%s()" must be of type array|bool|null, %s given.', __METHOD__, \is_object($options) ? \get_class($options) : \gettype($options))); | |||
| } | |||
|
|
|||
| $invalidOptions = \array_diff(\array_keys($options), $allowedOptions = [ | |||
| $endpoint = new Create(); | |||
| $invalidOptions = \array_diff(\array_keys($options), $allowedOptions = \array_merge($endpoint->getParamWhitelist(), [ | |||
There was a problem hiding this comment.
If I understand this correctly, it adds an additional call to Elasticsearch each time before an index is created? Assuming index creation is not happening too often, this is probably not a problem but worth mentioning.
There was a problem hiding this comment.
No extra call to Elasticsearch, just the valid options are retrieved from the underlying endpoint class.
There was a problem hiding this comment.
Ah, good to know. I somehow assumed it gets it from ES each time.
| @@ -422,7 +423,9 @@ public function create(array $args = [], $options = null): Response | |||
| } | |||
| } | |||
|
|
|||
| $endpoint = new Create(); | |||
| unset($options['recreate']); | |||
There was a problem hiding this comment.
I think I need to read up on my end what this flag does or doesn't.
Is it now with the new code that recreate or was this always the case and now we remove it but didn't before? Trying to understand which part is related to the change of getting the options and what is further improvements.
| @@ -402,9 +402,10 @@ public function create(array $args = [], $options = null): Response | |||
| throw new \TypeError(\sprintf('Argument 2 passed to "%s()" must be of type array|bool|null, %s given.', __METHOD__, \is_object($options) ? \get_class($options) : \gettype($options))); | |||
| } | |||
|
|
|||
| $invalidOptions = \array_diff(\array_keys($options), $allowedOptions = [ | |||
| $endpoint = new Create(); | |||
| $invalidOptions = \array_diff(\array_keys($options), $allowedOptions = \array_merge($endpoint->getParamWhitelist(), [ | |||
There was a problem hiding this comment.
Ah, good to know. I somehow assumed it gets it from ES each time.
|
Waiting for #1867 to be merged before merging this one as I will have to update the tests. |
deguif
force-pushed
the
create-index-options
branch
from
fc8cda4 to
0c398af
Compare
| $index->create([]); | ||
| $this->_waitForAllocation($index); | ||
| $index->create([], [ | ||
| 'wait_for_active_shards' => 'all', |
There was a problem hiding this comment.
Here's a concrete example of the use of native options when creating an index.
This allows to replace the use of $this->_waitForAllocation() by a native option.
This PR will allow to pass endpoint query parameters to index creation: https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-create-index.html#indices-create-api-query-params
The list of available options is retrieved from the corresponding elasticsearch endpoint class to avoid maintaining it here.