Skip to content

Make various LifecycleSettings Settings internal#32381

Merged
dakrone merged 5 commits intoelastic:index-lifecyclefrom
dakrone:ilm-make-lifecycle-settings-internal
Jul 30, 2018
Merged

Make various LifecycleSettings Settings internal#32381
dakrone merged 5 commits intoelastic:index-lifecyclefrom
dakrone:ilm-make-lifecycle-settings-internal

Conversation

@dakrone
Copy link
Copy Markdown
Member

@dakrone dakrone commented Jul 25, 2018

These are only ever set internally during regular ILM execution, they don't need
to be set otherwise.

A subsequent PR will work on adding a dedicated endpoint for the
LIFECYCLE_NAME setting so it can be changed by a user (and then marked as
InternalIndex as well)

Relates to #29823

These are only ever set internally during regular ILM execution, they don't need
to be set otherwise.

A subsequent PR will work on adding a dedicated endpoint for the
`LIFECYCLE_NAME` setting so it can be changed by a user (and then marked as
`InternalIndex` as well)

Relates to elastic#29823
@dakrone dakrone added the :Data Management/ILM+SLM DO NOT USE. Use ":StorageEngine/ILM" or ":Distributed Coordination/SLM" instead. label Jul 25, 2018
@dakrone dakrone requested review from colings86 and talevy July 25, 2018 20:14
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-infra

Copy link
Copy Markdown
Contributor

@talevy talevy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dakrone
Copy link
Copy Markdown
Member Author

dakrone commented Jul 25, 2018

@elasticmachine retest this please

Setting.Property.IndexScope, Setting.Property.NotCopyableOnResize, Setting.Property.InternalIndex);
public static final Setting<Boolean> LIFECYCLE_SKIP_SETTING = Setting.boolSetting(LIFECYCLE_SKIP, false,
Setting.Property.Dynamic, Setting.Property.IndexScope);
Setting.Property.Dynamic, Setting.Property.IndexScope, Setting.Property.InternalIndex);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This setting should not be internal as currently there is no API to set it other than the update settings API. I actually think that now we have the operation mode API we should remove this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I missed this. agreed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I assumed tests would fail, guess this wasn't tested thoroughly enough. I can remove this in a follow-up

Copy link
Copy Markdown
Contributor

@colings86 colings86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dakrone dakrone merged commit dc97051 into elastic:index-lifecycle Jul 30, 2018
jasontedor pushed a commit that referenced this pull request Aug 17, 2018
These are only ever set internally during regular ILM execution, they don't need
to be set otherwise.

A subsequent PR will work on adding a dedicated endpoint for the
`LIFECYCLE_NAME` setting so it can be changed by a user (and then marked as
`InternalIndex` as well)

Relates to #29823
@dakrone dakrone deleted the ilm-make-lifecycle-settings-internal branch February 4, 2019 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Data Management/ILM+SLM DO NOT USE. Use ":StorageEngine/ILM" or ":Distributed Coordination/SLM" instead.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants