Skip to content

Use Azure upload method instead of our own implementation (#26751)#26839

Merged
dadoonet merged 1 commit intoelastic:5.6from
dadoonet:pr/26751-azure-upload-56
Oct 3, 2017
Merged

Use Azure upload method instead of our own implementation (#26751)#26839
dadoonet merged 1 commit intoelastic:5.6from
dadoonet:pr/26751-azure-upload-56

Conversation

@dadoonet
Copy link
Copy Markdown
Contributor

  • Use Azure upload method instead of our own implementation

We are not following the Azure documentation about uploading blobs to Azure storage. https://docs.microsoft.com/en-us/azure/storage/blobs/storage-java-how-to-use-blob-storage#upload-a-blob-into-a-container

Instead we are using our own implementation which might cause some troubles and rarely some blobs can be not immediately commited just after we close the stream. Using the standard implementation provided by Azure team should allow us to benefit from all the magic Azure SDK team already wrote.

And well... Let's just read the doc!

  • Adapt integration tests

  • Simplify all the integration tests and extends ESBlobStoreRepositoryIntegTestCase tests

    • removes IT testForbiddenContainerName() as it is useless. The plugin does not create anymore the container but expects that the user has created it before registering the repository
    • merges 2 IT classes so all IT tests are ran from one single class
    • We don't remove/create anymore the container between each single test but only for the test suite

Backport of #26751 in 5.6 branch

)

* Use Azure upload method instead of our own implementation

We are not following the Azure documentation about uploading blobs to Azure storage. https://docs.microsoft.com/en-us/azure/storage/blobs/storage-java-how-to-use-blob-storage#upload-a-blob-into-a-container

Instead we are using our own implementation which might cause some troubles and rarely some blobs can be not immediately commited just after we close the stream. Using the standard implementation provided by Azure team should allow us to benefit from all the magic Azure SDK team already wrote.

And well... Let's just read the doc!

* Adapt integration tests
* Simplify all the integration tests and extends ESBlobStoreRepositoryIntegTestCase tests

    * removes IT `testForbiddenContainerName()` as it is useless. The plugin does not create anymore the container but expects that the user has created it before registering the repository
   * merges 2 IT classes so all IT tests are ran from one single class
   * We don't remove/create anymore the container between each single test but only for the test suite

Backport of elastic#26751 in 5.6 branch
@dadoonet
Copy link
Copy Markdown
Contributor Author

@imotov In case you'd like to give a review but it's basically #26751 for 5.6 branch.
Tested locally and worked well (including Integration tests against azure platform).
Just waiting for CI to finish that everything is correct.

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. Sorry for the delay.

@dadoonet dadoonet merged commit f1434c8 into elastic:5.6 Oct 3, 2017
@dadoonet dadoonet deleted the pr/26751-azure-upload-56 branch October 3, 2017 13:26
@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

>bug :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v5.6.3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants