Skip to content

Simplify Azure blobStore method signatures#26752

Merged
dadoonet merged 5 commits intoelastic:masterfrom
dadoonet:pr/azure-repo-reuse-container-name
Oct 4, 2017
Merged

Simplify Azure blobStore method signatures#26752
dadoonet merged 5 commits intoelastic:masterfrom
dadoonet:pr/azure-repo-reuse-container-name

Conversation

@dadoonet
Copy link
Copy Markdown
Contributor

While working on #26751, I found that we are passing the container name on every single method although we don't need it as it is stored within the blobstore object already.

This PR simplifies a bit that part of the code.

While working on elastic#26751, I found that we are passing the container name on every single method although we don't need it as it is stored within the blobstore object already.

This PR simplifies a bit that part of the code.
Copy link
Copy Markdown
Contributor

@imotov imotov left a comment

Choose a reason for hiding this comment

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

LGTM.

}

public void deleteFiles(String container, String path) throws URISyntaxException, StorageException
public void deleteFiles(String path) throws URISyntaxException, StorageException
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 don't think this method is used anymore. So, maybe you should clean it up as well while you are at it.

@dadoonet dadoonet removed the review label Sep 25, 2017
# Conflicts:
#	plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobContainer.java
#	plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java
Was not used anymore.
Also move some properties in AzureBlobContainer to `private` members.
Copy link
Copy Markdown
Contributor

@imotov imotov left a comment

Choose a reason for hiding this comment

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

Still LGTM

@dadoonet
Copy link
Copy Markdown
Contributor Author

dadoonet commented Oct 4, 2017

@elasticmachine test it please

@dadoonet dadoonet merged commit 84a3899 into elastic:master Oct 4, 2017
@dadoonet dadoonet deleted the pr/azure-repo-reuse-container-name branch October 4, 2017 18:17
@dadoonet dadoonet added the v6.1.0 label Oct 4, 2017
dadoonet added a commit that referenced this pull request Oct 4, 2017
While working on #26751, I found that we are passing the container name on every single method although we don't need it as it is stored within the blobstore object already.

This commit simplifies a bit that part of the code.

It also removes `repositoryName` from AzureBlobStore which was not used anymore.
Also we move some properties in AzureBlobContainer to `private` members.

Backport of #26752 in 6.x branch
@dadoonet
Copy link
Copy Markdown
Contributor Author

dadoonet commented Oct 4, 2017

Thanks @imotov.

I pushed it in master and in 6.x with 6cb6035.

@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 >non-issue v6.1.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants