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

Confusion over codec parameter to ILM force-merge action #57034

Closed
DaveCTurner opened this issue May 21, 2020 · 7 comments
Closed

Confusion over codec parameter to ILM force-merge action #57034

DaveCTurner opened this issue May 21, 2020 · 7 comments
Labels
>bug :Data Management/ILM+SLM Index and Snapshot lifecycle management >docs General docs changes Team:Data Management Meta label for data/management team Team:Docs Meta label for docs team

Comments

@DaveCTurner
Copy link
Contributor

DaveCTurner commented May 21, 2020

In #49974 we added support for changing to the best_compression codec in ILM's force-merge action, but it seems that the parameter in the REST API is actually index_codec and not codec as per the docs:

public static final ParseField CODEC = new ParseField("index_codec");

This means the docs for 7.7 are wrong (see #55978). I think index_codec is an unfortunate choice of name and it would be preferable to change it to codec even though it's now a breaking change so we'd have to do the whole deprecation dance to achieve that.

@DaveCTurner DaveCTurner added >bug :Data Management/ILM+SLM Index and Snapshot lifecycle management Team:Data Management Meta label for data/management team labels May 21, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/ILM+SLM)

@dakrone
Copy link
Member

dakrone commented May 26, 2020

I think index_codec is an unfortunate choice of name and it would be preferable to change it to codec

I'm going to guess (since it was a community PR), that index_codec came from it being the index.codec setting, and I asked to not have a dot in the field name for the action definition.

Can you explain why you think index_codec is an unfortunate name? Just parity between the two?

@DaveCTurner
Copy link
Contributor Author

Yes, just that I think of the index setting as simply codec (the namespace may be given separately and can even be omitted in some contexts) and that the index_ prefix seems redundant (whose codec could we be changing if not the index's?). I'm not certain we should rename it, but I do expect this discrepancy will catch me out in the future.

@zuozp8
Copy link

zuozp8 commented Aug 11, 2020

@jrodewig jrodewig added the >docs General docs changes label Sep 10, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs (>docs)

@jrodewig
Copy link
Contributor

jrodewig commented Sep 10, 2020

#62243 updates the docs to use the current index_codec param name.

@jrodewig
Copy link
Contributor

@dakrone @DaveCTurner I'm going to close this issue as the docs are now fixed. Please re-open if you feel we need to rename the parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Data Management/ILM+SLM Index and Snapshot lifecycle management >docs General docs changes Team:Data Management Meta label for data/management team Team:Docs Meta label for docs team
Projects
None yet
Development

No branches or pull requests

5 participants