Make PUT consistent for `_mapping`, `_alias` and `_warmer` #4687

brwe · 2014-01-10T14:46:53Z

This pull request makes consistent the PUT for _mapping, _alias and _warmer as described in issue #4071.

`_mapping`:

Single type can now be added with

[PUT|POST] {index|_all|*|regex|blank}/[_mapping|_mappings]/type

and

[PUT|POST] {index|_all|*|regex|blank}/type/[_mapping|_mappings]

`_warmer`

A single warmer can now be put with

[PUT|POST] {index|_all|*|prefix*|blank}/{type|_all|*|prefix*|blank}/[_warmer|_warmers]/warmer_name

`_alias`

A single alias can now be PUT with

[PUT|POST] {index|_all|*|prefix*|blank}/[_alias|_aliases]/alias

brwe · 2014-01-10T17:11:21Z

docs/reference/indices/aliases.asciidoc

@@ -158,12 +158,13 @@ curl -XGET 'http://localhost:9200/alias2/_search?q=user:kimchy&routing=2,3'
 There is also an api to add a single index alias, with options: 

 [horizontal]
-`index`::   The index to alias refers to. This is a required option. 
+`index`::   The index to alias refers to. Can be any of `blank | * | _all | regex | name1, name2, …` 


regex -> glob

kimchy · 2014-01-11T02:10:17Z

LGTM, would be great if @clintongormley can review the rest tests and the behavior as well...

brwe · 2014-01-13T23:11:57Z

I updated the pull request and added also the changes for DELETE. I think it makes sense to squash the commits for DELETE and PUT _alias (7c9f814, 7188455) because they both touch the same files.

imotov · 2014-01-13T23:17:00Z

src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java

-        out.writeString(index);
-        out.writeString(alias);
+        out.writeOptionalString(index);
+        out.writeOptionalString(alias);
        if (filter == null) {


Maybe replace these with writeOptionalString as well while you are at it

dakrone · 2014-01-14T00:00:12Z

The output from putting a mapping with no type specified seem strange:

pre-this-change:

curl -XDELETE 'localhost:9200/test4687'
echo
curl -XPOST 'localhost:9200/test4687'
echo
curl -XPOST 'localhost:9200/test4687/_mapping' -d'{
  "_source": {"enabled": false},
  "properties": {
    "body": {"type": "string"},
    "newtype": {"type": "integer"}
  }
}'
echo
curl 'localhost:9200/test4687/_mapping?pretty'

Output:

{"acknowledged":true}
{"acknowledged":true}
{"error":"ActionRequestValidationException[Validation Failed: 1: mapping type is missing;]","status":500}
{
  "test4687" : { }
}

post-this-change:

curl -XDELETE 'localhost:9200/test4687'
echo
curl -XPOST 'localhost:9200/test4687'
echo
curl -XPOST 'localhost:9200/test4687/_mapping' -d'{
  "_source": {"enabled": false},
  "properties": {
    "body": {"type": "string"},
    "newtype": {"type": "integer"}
  }
}'
echo
curl 'localhost:9200/test4687/_mapping?pretty'

Output:

{"acknowledged":true}
{"acknowledged":true}
{"acknowledged":true}
{
  "test4687" : {
    "test4687" : {
      "_source" : {
        "enabled" : false
      },
      "properties" : {
        "body" : {
          "type" : "string"
        },
        "newtype" : {
          "type" : "integer"
        }
      }
    }
  }
}

Is this intended? The double-wrapping of types in the post-change seems incorrect to me.

jpountz · 2014-01-14T10:00:53Z

docs/reference/indices/aliases.asciidoc

-There is also an api to add a single index alias, with options: 
+An alias can also be added with the endpoint
+
+`[PUT|POST] /{index}/_alias/{name}`


Even if we happen to support POST for doing that, should we rather document only PUT to make it more REST-ish?

brwe · 2014-01-14T11:18:09Z

@dakrone about the double wrapping: when you call

curl -XPOST 'localhost:9200/test4687'

this creates and index named test4687

When you call

curl -XPOST 'localhost:9200/test4687/_mapping'

this creates the type test4687 for all indices. The reason is that we would like to let people leave the index blank when putting a mapping. The original way to do this was

PUT /index/type

but since we now can leave the index blank this means that when using the uri

/something/_mapping

something will be interpreted as type.

If this is too confusing maybe we should remove the old uris

/index/type/_mapping

and

/index/_mapping?type=name

?

clintongormley · 2014-01-14T11:22:37Z

@brwe Agreed that is confusing. Would it be possible to only support the old URLs by making {index} and {type} required in those cases?

clintongormley · 2014-01-14T11:24:54Z

I'd like to keep the old URLs around, to make migration easier, but if we can't do the above, then I'd be leaning towards making {index} required for all PUT operations (incl _settings, unfortunately). It can still be * or _all, but just not blank. (but if you can just make it required for the old URLs, then I'd prefer that)

brwe · 2014-01-14T12:39:33Z

@clintongormley @dakrone ok, it is easy to remove the options and only keep the old functionality. Should we then remove PUT /{index}/{type}/_mapping from the documentation? It would be quite confusing to explain why for PUT /{index}/_mapping/{type} we can leave index blank but not for the old path.

s1monw · 2014-01-14T16:54:52Z

src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java

        if (filter == null) {
            out.writeBoolean(false);
        } else {
            out.writeBoolean(true);
-            out.writeString(filter);


these replacements seem to be wrong the if / else logic is obsolete now

right, will remove

brwe · 2014-01-14T18:17:47Z

@imotov @dakrone @s1monw @clintongormley Thanks so much for the review!

I updated the pull request with the following changes:

new commits with the changes requested in review
commit 9072c89068 : 404 if aliases are reqested to be deleted but none of the aliases was found, this also affects POST /_aliases { actions: [...]} if the actions contains only deletes
added all commits in @clintongormley pull request Issue 4071 put brwe/elasticsearch#1

Next, I will squash and push.

See issue elastic#4071 PUT options for _mapping: Single type can now be added with `[PUT|POST] {index|_all|*|regex|blank}/[_mapping|_mappings]/type` and `[PUT|POST] {index|_all|*|regex|blank}/type/[_mapping|_mappings]` PUT options for _warmer: PUT with a single warmer can now be done with `[PUT|POST] {index|_all|*|prefix*|blank}/{type|_all|*|prefix*|blank}/[_warmer|_warmers]/warmer_name` PUT options for _alias: Single alias can now be PUT with `[PUT|POST] {index|_all|*|prefix*|blank}/[_alias|_aliases]/alias` DELETE options _mapping: Several mappings can be deleted at once by defining several indices and types with `[DELETE] /{index}/{type}` `[DELETE] /{index}/{type}/_mapping` `[DELETE] /{index}/_mapping/{type}` where `index= * | _all | glob pattern | name1, name2, …` `type= * | _all | glob pattern | name1, name2, …` Alternatively, the keyword `_mapings` can be used. DELETE options for _warmer: Several warmers can be deleted at once by defining several indices and names with `[DELETE] /{index}/_warmer/{type}` where `index= * | _all | glob pattern | name1, name2, …` `type= * | _all | glob pattern | name1, name2, …` Alternatively, the keyword `_warmers` can be used. DELETE options for _alias: Several aliases can be deleted at once by defining several indices and names with `[DELETE] /{index}/_alias/{type}` where `index= * | _all | glob pattern | name1, name2, …` `type= * | _all | glob pattern | name1, name2, …` Alternatively, the keyword `_aliases` can be used.

s1monw · 2014-01-14T19:01:18Z

LGMT +1 to push Good Job!!

brwe · 2014-01-16T12:04:36Z

pushed to master (411739f)

brwe reviewed Jan 10, 2014
View reviewed changes

imotov reviewed Jan 13, 2014
View reviewed changes

jpountz reviewed Jan 14, 2014
View reviewed changes

s1monw reviewed Jan 14, 2014
View reviewed changes

brwe closed this Jan 16, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make PUT consistent for `_mapping`, `_alias` and `_warmer` #4687

Make PUT consistent for `_mapping`, `_alias` and `_warmer` #4687

brwe commented Jan 10, 2014

brwe Jan 10, 2014

kimchy commented Jan 11, 2014

brwe commented Jan 13, 2014

imotov Jan 13, 2014

brwe Jan 14, 2014

dakrone commented Jan 14, 2014

jpountz Jan 14, 2014

clintongormley Jan 14, 2014

brwe Jan 14, 2014

brwe commented Jan 14, 2014

clintongormley commented Jan 14, 2014

clintongormley commented Jan 14, 2014

brwe commented Jan 14, 2014

s1monw Jan 14, 2014

brwe Jan 14, 2014

brwe commented Jan 14, 2014

s1monw commented Jan 14, 2014

brwe commented Jan 16, 2014

Make PUT consistent for _mapping, _alias and _warmer #4687

Make PUT consistent for _mapping, _alias and _warmer #4687

Conversation

brwe commented Jan 10, 2014

_mapping:

_warmer

_alias

Choose a reason for hiding this comment

kimchy commented Jan 11, 2014

brwe commented Jan 13, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dakrone commented Jan 14, 2014

pre-this-change:

post-this-change:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brwe commented Jan 14, 2014

clintongormley commented Jan 14, 2014

clintongormley commented Jan 14, 2014

brwe commented Jan 14, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brwe commented Jan 14, 2014

s1monw commented Jan 14, 2014

brwe commented Jan 16, 2014

Make PUT consistent for `_mapping`, `_alias` and `_warmer` #4687

Make PUT consistent for `_mapping`, `_alias` and `_warmer` #4687

`_mapping`:

`_warmer`

`_alias`