Skip to content

Add azure storage endpoint suffix #26432#26568

Merged
rjernst merged 6 commits intoelastic:masterfrom
liketic:feature/azure-china
Sep 21, 2017
Merged

Add azure storage endpoint suffix #26432#26568
rjernst merged 6 commits intoelastic:masterfrom
liketic:feature/azure-china

Conversation

@liketic
Copy link
Copy Markdown

@liketic liketic commented Sep 10, 2017

Close #26432

Happy to hear any comments to make this PR better. Thanks in advance. 😄

@elasticmachine
Copy link
Copy Markdown
Collaborator

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?

Copy link
Copy Markdown
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

I left a question.


/**
* Azure endpoint suffix. Default to core.windows.net (CloudStorageAccount.DEFAULT_DNS).
*/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why does this need to be a secure setting?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks! I think a normal string is OK. Shall I merge with master for review?

/**
* Azure endpoint suffix. Default to core.windows.net (CloudStorageAccount.DEFAULT_DNS).
*/
public static final Setting<String> ENDPOINT_SUFFIX_SETTING = Setting.affixKeySetting(PREFIX, "endpoint_suffix",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think the endpoint needs to be a secure setting.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, of course. Thanks.

@liketic
Copy link
Copy Markdown
Author

liketic commented Sep 15, 2017

@jasontedor @rjernst I updated the PR, please review again. Thanks!

@rjernst
Copy link
Copy Markdown
Member

rjernst commented Sep 15, 2017

@elasticmachine please test this

@rjernst
Copy link
Copy Markdown
Member

rjernst commented Sep 18, 2017

@elasticmachine test this

@liketic
Copy link
Copy Markdown
Author

liketic commented Sep 19, 2017

@rjernst Thanks, I think the testing errors are not caused by my changes...

@rjernst
Copy link
Copy Markdown
Member

rjernst commented Sep 20, 2017

@elasticmachine retest this please

@rjernst
Copy link
Copy Markdown
Member

rjernst commented Sep 21, 2017

I tested this locally. Looks good, thanks @liketic!

@rjernst rjernst merged commit 601be4f into elastic:master Sep 21, 2017
rjernst pushed a commit that referenced this pull request Sep 21, 2017
Allow specifying azure storage endpoint suffix for an azure client.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Sep 21, 2017
* master:
  Add permission checks before reading from HDFS stream (elastic#26716)
  muted test
  [Docs] Fixed typo of *configuration* (elastic#25058)
  Add azure storage endpoint suffix elastic#26432 (elastic#26568)
@liketic liketic deleted the feature/azure-china branch October 26, 2017 02:48
@clintongormley clintongormley added :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs and removed :Plugin Repository Azure labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement v6.1.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants