Expand wildcards in snapshot #9903
Conversation
imotov
added
>breaking
v2.0.0-beta1
review
v1.5.0
:Distributed/Snapshot/Restore
|
looks good, left a few comments. I think we need to drop the indices options parsing code from multi_search too and reuse this new common method there too. |
|
One more thing, I would split this PR in two, the actual change and the breaking part that removes the old parameters, otherwise this whole change ends up in the 1.x release notes as a breaking one, which it isn't. |
imotov
force-pushed
the
issue-6097-expand-wildcards-in-snapshot
branch
from
7abe1ed
to
802b826
Compare
|
@javanna I pushed the changes based on your comments. I will open a separate PR for removal of old parameters from 2.0 once this PR is merged. |
| * If the node represents an array the corresponding array of strings is returned. | ||
| * Otherwise the node is treated as a comma-separated string. | ||
| */ | ||
| public static String[] nodeStringArrayValue(Object node) { |
There was a problem hiding this comment.
I find the node wording confusing, it makes me think of elasticsearch nodes. Maybe xContentNode?
There was a problem hiding this comment.
This is a convention of all static methods that are used to parse XContentMapValues.
There was a problem hiding this comment.
ok sorry I am not too familiar with these methods :)
|
Looks good @imotov , I think we can remove the breaking label? It might be good to add some more tests where missing around the apis that this PR touches using the different indices options, just to make sure that we still parse them correctly? I have the feeling that we have no extensive tests around that. What do you think? |
|
@javanna I pushed additional tests as we discussed. Could you take a look? |
|
thanks for adding those tests @imotov LGTM |
imotov
force-pushed
the
issue-6097-expand-wildcards-in-snapshot
branch
from
a011c31
to
7bd4654
Compare
clintongormley
changed the title
This PR makes handling of expand_wildcards parameters consistent. The first commit f5d8ca8 will go to both 1.x and master and the second commit 7abe1ed will go into master only. Closes #6097.