Skip to content
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.

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

IndexSettings NumberOfShards and NumberOfReplicas are objects, not int/long as expected #8248

Open
andersosthus opened this issue Jun 20, 2024 · 4 comments
Labels
8.x Relates to 8.x client version Area: Specification Category: Bug

Comments

@andersosthus
Copy link
Contributor

Elastic.Clients.Elasticsearch version: 8.14.3

Elasticsearch version: 8.14.1

.NET runtime version: .net8

Operating system version: Ubuntu 20.04

Description of the problem including expected versus actual behavior:
When loading index settings with ElasticsearchClient.Indicies.GetAsync the resulting IndexSettings.Settings object has NumberOfShards and NumberOfReplicas defined as object? and gets populated with the string representation of the value.

I would expect these fields to be int (not even nullable because I believe they are always present?)

Steps to reproduce:

  1. Create index: curl -X PUT http://localhost:9200/testindex
  2. Get IndexSettings:
var client = new ElasticsearchClient(new Uri("http://localhost:9200"));
var indexReq = new GetIndexRequest("testindex");
var getResponse = await client.Indices.GetAsync(indexReq);
  1. Inspect the response and look at Settings.NumberOfShards and Settings.NumberOfReplicas. See attached screenshot.

Expected behavior
NumberOfShards and NumberOfReplicas should be of type int.

image

@andersosthus andersosthus added 8.x Relates to 8.x client version Category: Bug labels Jun 20, 2024
@flobernd
Copy link
Member

Hi @andersosthus, the IndexSettings are not completely modelled in the specification at the moment.

This is how it currently looks for these 2 fields:

  /** @server_default 1 */
  number_of_shards?: integer | string // TODO: should be only int
  /** @server_default 0 */
  number_of_replicas?: integer | string // TODO: should be only int

As .NET does not support unions discriminated by type, the generator simplifies this construct as object. As you can see in your screenshot, the values are actually not ints, but strings containing a number.

The specification should probably be changed like this:

  /** @server_default 1 */
  number_of_shards?: Stringified<integer>
  /** @server_default 0 */
  number_of_replicas?: Stringified<integer>

@flobernd
Copy link
Member

Some additional context here.

Sadly, there won't be a fix for this before the above mentioned feature is implemented.

@andersosthus
Copy link
Contributor Author

Ah gotcha. I'll manage for now by wrapping them in a Convert.ToInt32() so no rush.

Question: According to your spec above, they are defined as nullable. Is this correct? I thought they were always present in the Index Settings?

@flobernd
Copy link
Member

I think the fields are all nullable at the moment, to make sure some of the other clients don't break if a field is not present. The Java client e.g. fails to deserialize objects, if a required field is missing. The Elasticsearch server on the other hand tends to omit certain fields under certain shady conditions 😋 Modelling it like this was probably the most safe way - this surely must be checked in detail as well as soon as we get to improve the IndexSettings in the spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.x Relates to 8.x client version Area: Specification Category: Bug
Projects
None yet
Development

No branches or pull requests

2 participants