-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Support for supplying index settings on restore #1362 #1372
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love moving this out of the attributes 👍 suggestion on different name UpdatableSettings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Mpdreamz much more better name 👍 . Maybe we should split this class into three? UpdatableSettings, UnmodifiableSettings and UnremovableSettings? https://github.com/elastic/elasticsearch/blob/master/src/main/java/org/elasticsearch/snapshots/RestoreService.java#L92-105
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any settings in SettingsNames that are not Updatable though, maybe I need another coffee 😄
Where would we use Unmodifiable and Unremovable ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's true, looks like I need second coffee :)
I'll change name to UpdatableSettings later on.
|
PR looks awesome as usual @robertlyson 👍 left a small comment. |
…gNames has been renamed to UpdatableSettings
|
Thanks for review. I've pushed change. |
Support for supplying index settings on restore #1362
|
ty @robertlyson ! |
PR for #1362 issue.
For
IndexSettingsproperty onIRrestoreDescriptorI reusedIUpdateSettingsRequest, but I'm not sure if this name fits in this context. Way how we can combineRestoreDescriptorwithUpdateSettingsDescriptoris really nice.Moreover, I've extracted all setting names to separate class. Avoiding googling for each plain index setting is really convenient.
Any thoughts? Let me know.
Thanks.