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

Projects
None yet
3 participants
@s1monw
Copy link
Contributor

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 s1monw added breaking labels Sep 3, 2014

@s1monw

This comment has been minimized.

Copy link
Contributor Author

s1monw commented Sep 3, 2014

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

@imotov

View changes

src/main/java/org/elasticsearch/index/snapshots/blobstore/BlobStoreIndexShardRepository.java Outdated
try {
snapshot = readSnapshot(blobContainer.readBlobFully(snapshotBlobName(snapshotId)));
try (InputStream stream = blobContainer.openInput(snapshotBlobName(snapshotId))) {
snapshot = readSnapshot(ByteStreams.toByteArray(stream));

This comment has been minimized.

Copy link
@imotov

imotov Sep 3, 2014

Member

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

@imotov

View changes

src/test/java/org/elasticsearch/common/blobstore/BlobStoreTest.java Outdated
assertArrayEquals(data, Arrays.copyOfRange(target.bytes, target.offset, target.length));
}

protected BlobContainer newBlobContiner(Random random) {

This comment has been minimized.

Copy link
@imotov

imotov Sep 3, 2014

Member

Continer is misspelled. Is random here for future expandability?

@imotov

This comment has been minimized.

Copy link
Member

imotov commented Sep 3, 2014

Left a couple of comments. Everything else LGTM.

@s1monw

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

imotov commented Sep 5, 2014

LGTM

[STORE] Simplify reading / writing from and to BlobContainer
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 force-pushed the s1monw:cleanup_blob_store branch to 7f32e8c Sep 5, 2014

@s1monw s1monw merged commit 7f32e8c into elastic:master Sep 5, 2014

s1monw added a commit that referenced this pull request Sep 5, 2014

[STORE] Simplify reading / writing from and to BlobContainer
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 s1monw:cleanup_blob_store branch Sep 5, 2014

s1monw added a commit that referenced this pull request Sep 8, 2014

[STORE] Simplify reading / writing from and to BlobContainer
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

BlobContainer interface changed in elasticsearch 1.4.0
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

BlobContainer interface changed in elasticsearch 1.4.0
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

BlobContainer interface changed in elasticsearch 1.4.0
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

BlobContainer interface changed in elasticsearch 1.4.0
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

Rest: remove old SNAPSHOT_DATA alias (sd)
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

Rest: remove old SNAPSHOT_DATA alias (sd)
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

Rest: remove old SNAPSHOT_DATA alias (sd)
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
You can’t perform that action at this time.