File settings health indicator

[prdoyle]

Describes the health indicator added in elastic/elasticsearch#117081.

[github-actions]

Following you can find the validation results for the API you have changed.

API	Status	Request	Response
health_report	🟢	4/4	4/4

You can validate this API yourself by using the make validate target.

[dakrone]

This looks suspiciously small, I seem to recall in the past these included the generated outputs in git. Did you run the commands in https://github.com/elastic/elasticsearch-specification?tab=readme-ov-file#how-to-generate-the-json-representation after adding this?

[prdoyle]

No I did not do that. Thanks for the pointer!

[github-actions]

Following you can find the validation results for the API you have changed.

API	Status	Request	Response
health_report	🟢	4/4	4/4

You can validate this API yourself by using the make validate target.

[prdoyle]

One thing that puzzles me. Most indicators use HealthIndicatorDetails.EMPTY under certain circumstances, which corresponds to JSON {}, yet they seem to declare the detail fields to be required (not optional). I'm not sure how this works, but I followed the same pattern.

[dakrone]

they seem to declare the detail fields to be required (not optional)

This sounds like a bug to me. The details field should be optional because hitting GET /_health_report?verbose=false does not include the details subobject. We should fix this. /cc @pquentin so that you are aware of this.

[prdoyle]

This sounds like a bug to me. The details field should be optional because hitting GET /_health_report?verbose=false does not include the details subobject. We should fix this. /cc @pquentin so that you are aware of this.

To be clear, the details field itself is optional; but the fields of the details object are not optional despite the fact that HealthIndicatorDetails.EMPTY is serialized as {} with no fields, according to this code.

[prdoyle]

I think I figured it out. According to this code, we skip the details field if the value equals HealthIndicatorDetails.EMPTY. This means the {} value is ignored and the field is skipped.

Given this, I think my change here is actually fine. FYI @pquentin

I will say: I agree with @dakrone that this sounds like a bug, which isn't ideal.

[pquentin]

Thanks! LGTM.

Indeed, if details is either absent or contains the full details, then this change is correct. Thank you @prdoyle for the pull request and @dakrone for the review.