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

Remove setting copy settings on resize to false #30321

Closed

Conversation

jasontedor
Copy link
Member

@jasontedor jasontedor commented May 2, 2018

As the copy_settings REST parameter and copySettings transport field was deprecated in 6.4.0 slated for removal in 8.0.0, this commit puts us farther down that path by:

  • defaulting copy settings to true
  • preventing settings copy settings to false

Relates #28347, relates #30255

As the copy_settings REST parameter and copySettings transport field was
deprecated in 6.4.0 slated for removal in 8.0.0, this commit puts us
farther down that path by:
 - defaulting copy settings to true
 - preventing settings copy settings to false
@jasontedor jasontedor added >breaking review :Data Management/Indices APIs APIs to create and manage indices and templates v7.0.0 labels May 2, 2018
@jasontedor jasontedor requested review from colings86 and talevy May 2, 2018 03:26
@colings86
Copy link
Contributor

@jasontedor This process for removal is a little different than I envisaged. I had thought that we would:

  • Add copySettings in 6.4.0 defaulted to false and add a deprecation warning if copySettings was not provided stating that the behaviour will change in 7.0. the actual copySettings parameter would not be deprecated though since we want people to use it in 6.4-6.last
  • In 7.0, change the default of copySettings to true, and make it so we emit deprecation warnings if the parameter is explicitly set on a resize request explaining that the parameter will be removed in 8.0. We wouldn't restrict which values of copySettings can be used in 7.0 though, just emit the warning if it is used.
  • In 8.0, remove the copySettings parameter completely

The worry I have with the approach in this PR is that it puts the hard break in 7.0 since there is no way of not copying settings so to me it seems that its the same as just removing the parameter in 7.0?

@jasontedor
Copy link
Member Author

@colings86 I am following the plan that we outlined here: #28347

I am fine with changing the plan, but I think the place for that discussion is the source issue.

@colings86
Copy link
Contributor

@jasontedor I left a comment on #28347 with my comments from above so we can discuss. Sorry I didn't think about this and bring up the concern earlier

@hub-cap hub-cap added :Data Management/Indices APIs APIs to create and manage indices and templates and removed :Data Management/Indices APIs APIs to create and manage indices and templates labels May 3, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jasontedor
Copy link
Member Author

Superseded by #30404

@jasontedor jasontedor closed this May 6, 2018
@jasontedor jasontedor deleted the resize-settings-no-false branch May 6, 2018 22:23
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants