[ILM] Fix set replicas serialization and validation #85233
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jloleysens
added
Feature:ILM
v8.0.0
Team:Kibana Management
|
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
alisonelizabeth
approved these changes
Dec 8, 2020
There was a problem hiding this comment.
Thanks for fixing this @jloleysens! Tested locally. LGTM. Left a couple nits in the code.
| @@ -142,7 +142,7 @@ export const ColdPhase: FunctionComponent = () => { | |||
| { defaultMessage: 'Set replicas' } | |||
| ), | |||
| initialValue: Boolean( | |||
| policy.phases.cold?.actions?.allocate?.number_of_replicas | |||
| policy.phases.cold?.actions?.allocate?.number_of_replicas != null | |||
There was a problem hiding this comment.
nit: I don't think this needs to be wrapped in Boolean() now
|
|
||
| // First copy over all non-allocate and migrate actions. | ||
| const actions: SerializedActionWithAllocation = { ...otherActions }; | ||
|
|
||
| // The UI only knows about include, exclude and require, so copy over all other values. | ||
| // The UI only knows about include, exclude and require and number_of_replicas so copy over all other values. |
There was a problem hiding this comment.
Suggested change
| // The UI only knows about include, exclude and require and number_of_replicas so copy over all other values. | |
| // The UI only knows about include, exclude, require and number_of_replicas so copy over all other values. |
| /** | ||
| * The number of replicas value to set in the allocate action. | ||
| */ | ||
| nrOfReplicas?: number |
There was a problem hiding this comment.
nit: I think this might be a little more readable as numOfReplicas
💚 Build SucceededMetrics [docs]Async chunks
Distributable file count
History
To update your PR or re-run it, just comment with: |
jloleysens
deleted the
ilm/fix-set-replicas-serialization-and-validation
branch
jloleysens
added a commit
to jloleysens/kibana
that referenced
this pull request
Dec 8, 2020
* added fix for serializing replicas and updated validation to correctly check for non negative nr * added tests and fixed incorrect use of warm phase setting in cold phase * remove unused import * clean up use of Boolean and rename nrOfReplicas -> numberOfReplicas * fix comment
jloleysens
added a commit
that referenced
this pull request
Dec 9, 2020
* added fix for serializing replicas and updated validation to correctly check for non negative nr * added tests and fixed incorrect use of warm phase setting in cold phase * remove unused import * clean up use of Boolean and rename nrOfReplicas -> numberOfReplicas * fix comment
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Summarize your PR. If it involves visual changes include a screenshot or gif.
Address this comment (#85143 (review))
Fix #85226
Additionally, fixed validation for set replicas to allow the number
0and updated initial value for toggle logic to check for value existence, not truthiness or falsiness.Checklist