Simplify the BlobContainer blob writing interface#13434
Simplify the BlobContainer blob writing interface#13434imotov merged 1 commit intoelastic:masterfrom
Conversation
|
I think you should do this in a single PR but you can add different commits in it. I did not read in details for now but for sure it will simplify azure/s3 code! |
|
@dadoonet I just want to mention that this PR is not going to break S3 and Azure plugin. I have added backward compatibility layer for them. My main concern is that if I will dump all the changes into a single PR I will have hard time finding someone, who would be able to review all this changes. So, I am just trying to make things simpler. |
|
Ok. I missed that it's backward compatible. |
There was a problem hiding this comment.
InputStream or OutputStream? This is confusing because the method is InputStream readBlob(String blobName)
|
@imotov I looked at the code, it's LGTM I'm so so glad that you make this change, this will help a lot! Thank you so much |
|
I looked at the simplifications you mentioned for Azure and S3. I can easily find the added value for Azure as we don't need anymore to have That being said, I'm +1 for the change as it simplifies the interface and what a plugin author has basically to provide. |
Instead of asking blob store to create output for posting blob content, this change provides that content of the blob to the blob store for writing. This will significantly simplify the interface for S3 and Azure plugins.
8366e42 to
39ca450
Compare
`AbstractLegacyBlobContainer` was kept for historical reasons (see elastic#13434). We can migrate Azure and S3 repositories to use the new methods added in elastic#13434 so we can remove `AbstractLegacyBlobContainer` class.
Instead of asking blob store to create output for posting blob content, this change provides that content of the blob to the blob store for writing. This will significantly simplify the interface for S3 and Azure plugins.
I will open separate PRs for changes in S3 and Azure plugins to keep things simple.