add new phase definition setting used for retrieving phase to execute #33289
Conversation
talevy
added
WIP
:Data Management/ILM+SLM
|
Pinging @elastic/es-core-infra |
talevy
force-pushed
the
ilm-remember-phase
branch
2 times, most recently
from
95fe928 to
21252c6
Compare
Since policies can be updated independent of execution plans for the current phase being executed, it would be nice to know what the phase that is executing looks like in JSON. This PR does just that, while also using that index setting to recontruct the phase steps to execute (for consistency)
talevy
force-pushed
the
ilm-remember-phase
branch
from
21252c6 to
485be0c
Compare
| if (phase != null) { | ||
| phaseMap.put(currentPhase, phase); | ||
| } | ||
| policyToExecute = new LifecyclePolicy(currentPolicy.getType(), currentPolicy.getName(), phaseMap); |
There was a problem hiding this comment.
Why do we need to create a fake policy here with only 1 phase in it?
There was a problem hiding this comment.
the last step in the phase needs a correct nextstep, no? thinking this, and for the reason that step compilation only happens on the policy level, I thought it would be clear to just swap out the potential new version of the phase (or lack thereof) with the phase defined in settings.
I do think things in step compilation can be cleaned up to make this less wasteful and cleaner, but I thought it was reasonable
There was a problem hiding this comment.
@talevy and I spoke about this and although this will want to change at some point we can't change to compile directly from the phase right now whilst the PhaseAfterStep still exists (which is being removed in a different task).
| if (phase != null) { | ||
| phaseMap.put(currentPhase, phase); | ||
| } | ||
| policyToExecute = new LifecyclePolicy(currentPolicy.getType(), currentPolicy.getName(), phaseMap); |
There was a problem hiding this comment.
@talevy and I spoke about this and although this will want to change at some point we can't change to compile directly from the phase right now whilst the PhaseAfterStep still exists (which is being removed in a different task).
|
thanks for the review @colings86, took a bit for CI to pass |
…#33289) Since policies can be updated independent of execution plans for the current phase being executed, it would be nice to know what the phase that is executing looks like in JSON. This PR does just that, while also using that index setting to recontruct the phase steps to execute (for consistency)
3 participants
Since policies can be updated independent of execution plans for the current
phase being executed, it would be nice to know what the phase that is executing
looks like in JSON. This PR does just that, while also using that index setting
to reconstruct the phase steps to execute (for consistency)
There may or may not be a few unit tests missing to verify the json itself, let me know what you think. I figure the serialization is sufficient. Also, the expectation is that these will move from settings to IndexMetaData.Custom as a next step