Skip to content
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

Simplify reading / writing from and to BlobContainer #7551

Merged
merged 1 commit into from
Sep 5, 2014

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Sep 3, 2014

BlobContainer used to provide async APIs which are not used
internally. The implementation of these APIs are also not async
by nature and neither is any of the pluggable BlobContainers. This
commit simplifies the API to a simple input / output stream and
reduces the hierarchy of BlobContainer dramatically.
NOTE: This is a breaking change!

@s1monw
Copy link
Contributor Author

s1monw commented Sep 3, 2014

@imotov @kimchy would you mind taking a look at this

try {
snapshot = readSnapshot(blobContainer.readBlobFully(snapshotBlobName(snapshotId)));
try (InputStream stream = blobContainer.openInput(snapshotBlobName(snapshotId))) {
snapshot = readSnapshot(ByteStreams.toByteArray(stream));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not simply this even more by making readSnapshot to accept InputStream instead of array of bytes?

@imotov
Copy link
Contributor

imotov commented Sep 3, 2014

Left a couple of comments. Everything else LGTM.

@s1monw
Copy link
Contributor Author

s1monw commented Sep 4, 2014

@imotov I pushed a new commit including some more removals can you check it out?

@imotov
Copy link
Contributor

imotov commented Sep 5, 2014

LGTM

BlobContainer used to provide async APIs which are not used
internally. The implementation of these APIs are also not async
by nature and neither is any of the pluggable BlobContainers. This
commit simplifies the API to a simple input / output stream and
reduces the hierarchy of BlobContainer dramatically.
NOTE: This is a breaking change!

Closes elastic#7551
@s1monw s1monw merged commit 7f32e8c into elastic:master Sep 5, 2014
s1monw added a commit that referenced this pull request Sep 5, 2014
BlobContainer used to provide async APIs which are not used
internally. The implementation of these APIs are also not async
by nature and neither is any of the pluggable BlobContainers. This
commit simplifies the API to a simple input / output stream and
reduces the hierarchy of BlobContainer dramatically.
NOTE: This is a breaking change!

Closes #7551
@s1monw s1monw removed the review label Sep 5, 2014
@s1monw s1monw deleted the cleanup_blob_store branch September 5, 2014 20:30
s1monw added a commit that referenced this pull request Sep 8, 2014
BlobContainer used to provide async APIs which are not used
internally. The implementation of these APIs are also not async
by nature and neither is any of the pluggable BlobContainers. This
commit simplifies the API to a simple input / output stream and
reduces the hierarchy of BlobContainer dramatically.
NOTE: This is a breaking change!

Closes #7551
@clintongormley clintongormley changed the title [STORE] Simplify reading / writing from and to BlobContainer Internal: Simplify reading / writing from and to BlobContainer Sep 8, 2014
dadoonet added a commit to dadoonet/elasticsearch-cloud-azure that referenced this pull request Oct 22, 2014
AWS plugin needs an update because of this change elastic/elasticsearch#7551

Closes elastic#37.
dadoonet added a commit to dadoonet/elasticsearch-cloud-azure that referenced this pull request Oct 30, 2014
AWS plugin needs an update because of this change elastic/elasticsearch#7551

Closes elastic#37.
dadoonet added a commit to elastic/elasticsearch-cloud-azure that referenced this pull request Oct 30, 2014
AWS plugin needs an update because of this change elastic/elasticsearch#7551

Closes #37.

(cherry picked from commit 6d5ac76)
(cherry picked from commit e3028c1)
dadoonet added a commit to elastic/elasticsearch-cloud-azure that referenced this pull request Oct 30, 2014
AWS plugin needs an update because of this change elastic/elasticsearch#7551

Closes #37.

(cherry picked from commit 6d5ac76)
mikemccand added a commit that referenced this pull request Nov 25, 2014
SNAPSHOT_DATA was removed in #7551 but its alias (sd) was not.

This was fixed in master with #8643.
mikemccand added a commit that referenced this pull request Nov 25, 2014
SNAPSHOT_DATA was removed in #7551 but its alias (sd) was not.

This was fixed in master with #8643.
@clintongormley clintongormley changed the title Internal: Simplify reading / writing from and to BlobContainer Simplify reading / writing from and to BlobContainer Jun 6, 2015
mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
SNAPSHOT_DATA was removed in elastic#7551 but its alias (sd) was not.

This was fixed in master with elastic#8643.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants