-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Deprecate repositories.s3 settings #23278
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
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
jasontedor
left a comment
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 left a comment. I didn't do a full review.
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.
You can use ESTestCase#assertSettingDeprecations here.
|
@jasontedor I fixed it. It's strange that I had to fix other tests as it seems the assertWarning changed a bit. I'm going to try 5.x branch instead of mine to see if it's a regression introduced in 5.x or only something new in my branch. |
|
So apparently it's not only me: https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+5.3+multijob-intake/87/console |
|
I have seen the same failure quite a few times today on my box (5.x branch) |
rjernst
left a comment
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.
The change LGTM. You will need to update the latest master. See the changes Jason made there today to fix the tests.
b22af6c to
27d005d
Compare
|
Thanks @rjernst! @jasontedor as you requested the changes, can you review the last changes please? Thanks! |
|
@jasontedor I'm planning to merge this one hopefully next week. Any blocker on your side? Thanks! |
Global repositories settings we were able to set in elasticsearch config file under `repositories.s3` name space are now deprecated and will be removed in master (6.x). This includes `repositories.s3.bucket`, `repositories.s3.server_side_encryption`, `repositories.s3.buffer_size`, `repositories.s3.max_retries`, `repositories.s3.use_throttle_retries`, `repositories.s3.chunk_size`, `repositories.s3.compress`, `repositories.s3.storage_class`, `repositories.s3.canned_acl`, `repositories.s3.base_path` and `repositories.s3.path_style_access`. We must set those settings per repository instead. Respectively `bucket`, `server_side_encryption`, `buffer_size`, `max_retries`, `use_throttle_retries`, `chunk_size`, `compress`, `storage_class`, `canned_acl`, `base_path` and `path_style_access`. Related to elastic#22800
27d005d to
e8209b8
Compare
jasontedor
left a comment
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.
LGTM.
|
Thanks @jasontedor |
Global repositories settings we were able to set in elasticsearch config file under
repositories.s3name space are now deprecated and will be removed in master (6.x) - see #23276.
This includes
repositories.s3.bucket,repositories.s3.server_side_encryption,repositories.s3.buffer_size,repositories.s3.max_retries,repositories.s3.use_throttle_retries,repositories.s3.chunk_size,repositories.s3.compress,repositories.s3.storage_class,repositories.s3.canned_acl,repositories.s3.base_pathandrepositories.s3.path_style_access.We must set those settings per repository instead. Respectively
bucket,server_side_encryption,buffer_size,max_retries,use_throttle_retries,chunk_size,compress,storage_class,canned_acl,base_pathandpath_style_access.Related to #22800