Skip to content

Support boolean string for alias defination in CreateIndex API #120453

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

Support boolean string for alias defination in CreateIndex API #120453

wants to merge 6 commits into from

Conversation

@mushao999
Copy link
Contributor

@mushao999 mushao999 commented Jan 20, 2025

We found that boolean strings were not supported for alias defination (is_write_index & is_hidden) in RestCreateIndexAction. However, they are supported in RestIndexPutAliasAction. So I open this PR to allow RestCreateIndexAction to accept boolean string value.
Additional this PR also fix some small issues:

  • make KNOWN_FIELDS static to avoid creating it every time
  • make search_routing & index_routing rewrite routing field if set
  • throw IllegalArgumentException in RestIndexPutAliasAction to aovid slience Swallowing unsupport token

@elasticsearchmachine elasticsearchmachine added v9.0.0 external-contributor Pull request authored by a developer outside the Elasticsearch team needs:triage Requires assignment of a team area label labels Jan 20, 2025
@mushao999
Copy link
Contributor Author

mushao999 commented Jan 20, 2025

https://github.com/elastic/elasticsearch/blob/main/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestIndexPutAliasAction.java#L83-#L89
AND should we use ParseField#match here to make it same behavior as Alias#fromXcontent in dealing with the deprecatedNames

@AI-IshanBhatt AI-IshanBhatt added the :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. label Jan 20, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Indexing Meta label for Distributed Indexing team label Jan 20, 2025
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Jan 20, 2025

Pinging @elastic/es-distributed-indexing (Team:Distributed Indexing)

@elasticsearchmachine elasticsearchmachine removed the needs:triage Requires assignment of a team area label label Jan 20, 2025
@mushao999
Copy link
Contributor Author

mushao999 commented Jan 26, 2025 •
edited by leehinman
Loading

@talevy @javanna @dakrone please help to review this PR

@dakrone dakrone added :Data Management/Indices APIs APIs to create and manage indices and templates and removed :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. labels Jan 27, 2025
@elasticsearchmachine elasticsearchmachine added Team:Data Management Meta label for data/management team and removed Team:Distributed Indexing Meta label for Distributed Indexing team labels Jan 27, 2025
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Jan 27, 2025

Pinging @elastic/es-data-management (Team:Data Management)

@mushao999
Copy link
Contributor Author

mushao999 commented Feb 17, 2025

Pinging @dakrone

@dakrone
Copy link
Member

dakrone commented Feb 21, 2025

Thanks for the ping @mushao999, we discussed this internally as a team. We decided that for now we consider the leniency of allowing string-like booleans to be unfortunate, rather than a leniency to which we want to more closely align. If it were not a breaking change, we'd be more likely to change the alias API to be strict about boolean values, rather than make the create index API more lenient.

So for now, I'm going to close this PR, thanks for opening it though!

@dakrone dakrone closed this Feb 21, 2025
@mushao999 mushao999 mentioned this pull request Mar 13, 2025
Merged
elasticsearchmachine pushed a commit that referenced this pull request Apr 28, 2025
@mushao999
Throw exception for unsupported values type in Alias (#124737)
637807c 
After creating index with alias using the following request

```
PUT test-index
{
  "aliases": {
    "alias1": {
      "is_write_index": "true"
    }
  }
}
```

we got the following result for get index request:

```
{
  "test-index" : {
    "aliases" : {
      "alias1" : { }
    },
    "mappings" : { },
    "settings" : {   
      ...
    }
  }
}
```

The `is_write_index` field is missing because string boolean value is
not supported for this filed and `no warning message showed`, which will
mislead the users. In #120453 I open a PR to let the createIndex API
support string boolean values for `is_write_index` field, but @dakrone
think it's better to be strict about boolean values. So I open this PR
to let the Alias class throw exception for the unsupport value type to
avoid the slience swallowing of this case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

:Data Management/Indices APIs APIs to create and manage indices and templates external-contributor Pull request authored by a developer outside the Elasticsearch team Team:Data Management Meta label for data/management team v9.1.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mushao999 @elasticsearchmachine @dakrone @AI-IshanBhatt