Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
Closed #1090. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#event-1092913378
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
No changes are required in RegionScopedBlobStore. Working on the DynamicLargeObjectAPI. Will submit PR for that. Closing this PR. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#issuecomment-303282204
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul requested changes on this pull request. I have no idea what the latest update is supposed to do. There are spurious changes and no unit or integration tests. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#pullrequestreview-39563388
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
@archupsg03 pushed 1 commit. c1c338f Provider Changes -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/jclouds/jclouds/pull/1090/files/2fe1ef35fd4c65f6602b0f0b80466f6550e61331..c1c338f2d029441a894b87b55c8654f730360e5b
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul commented on this pull request. > + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jclouds.openstack.swift.v1.options; + +import com.google.common.util.concurrent.ListeningExecutorService; +import org.jclouds.blobstore.options.PutOptions; + +public class OpenStackSwiftPutOptions extends PutOptions { No, you still don't understand the difference between the provider API and the portable abstraction. Go compare something like B2 with its [MultipartApi](https://github.com/jclouds/jclouds/blob/master/providers/b2/src/main/java/org/jclouds/b2/features/MultipartApi.java) against [B2BlobStore](https://github.com/jclouds/jclouds/blob/master/providers/b2/src/main/java/org/jclouds/b2/blobstore/B2BlobStore.java). The former implements the REST API while the latter conforms to the `BlobStore` interface. This pull request should not contain any of the latter. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r117505244
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
@archupsg03 pushed 1 commit. 2fe1ef3 Build failure issue addressed -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/jclouds/jclouds/pull/1090/files/e4eff0dc0c167f6a3d4bdc654d4d9a12d85bbdd8..2fe1ef35fd4c65f6602b0f0b80466f6550e61331
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
@archupsg03 pushed 1 commit. e4eff0d Build failure issue addressed -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/jclouds/jclouds/pull/1090/files/297610fe9120f6e37b33810c2f673c3c5a256a99..e4eff0dc0c167f6a3d4bdc654d4d9a12d85bbdd8
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
@archupsg03 pushed 1 commit. c6f17c1 Comments Address - Except separation of higher and lower level abstraction -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/jclouds/jclouds/pull/1090/files/13bb2016abc6abb2a6d25fdf8dcb4c3517f692ca..c6f17c12d700658d2ec9ac4d29883cbd3edbfce6
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
archupsg03 commented on this pull request. > + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jclouds.openstack.swift.v1.options; + +import com.google.common.util.concurrent.ListeningExecutorService; +import org.jclouds.blobstore.options.PutOptions; + +public class OpenStackSwiftPutOptions extends PutOptions { I am almost done with rest of the changes, except the separation of lower and higher API. My understanding from the comment #1090, is that DynamicLargeObjetApi should have methods such as initiateMultiPartUpload, uploadMultiPart, completeMultiPart etc., Is this correct ? If not Can you just give an example, say what methods are expected to in DynamiCLargeObjectApi file ? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r117417979
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul commented on this pull request. > +* @param prefix +* +*/ + private void removeObjectsWithPrefix(String container, String prefix) { + String nextMarker = null; + ObjectApi objectApi = api.getObjectApi(regionId, container); + do { + org.jclouds.openstack.swift.v1.options.ListContainerOptions listContainerOptions = prefix(prefix); + if (nextMarker != null) { +listContainerOptions = listContainerOptions.marker(nextMarker); + } + PageSet chunks = list(container, listContainerOptions); + Iterator iter = chunks.iterator(); + + while (iter.hasNext()) { +objectApi.delete(iter.next().getName()); Yes. DLO generally is poorly thought and and supported. Again, scope this commit down to just the provider changes and submit the portable abstraction changes as a separate pull request. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r117285005
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
archupsg03 commented on this pull request. > +* @param prefix +* +*/ + private void removeObjectsWithPrefix(String container, String prefix) { + String nextMarker = null; + ObjectApi objectApi = api.getObjectApi(regionId, container); + do { + org.jclouds.openstack.swift.v1.options.ListContainerOptions listContainerOptions = prefix(prefix); + if (nextMarker != null) { +listContainerOptions = listContainerOptions.marker(nextMarker); + } + PageSet chunks = list(container, listContainerOptions); + Iterator iter = chunks.iterator(); + + while (iter.hasNext()) { +objectApi.delete(iter.next().getName()); Does DLO, bulk deletes are also expected to delete only the manifest file ? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r117208798
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
@archupsg03 The more I think about this, let's work on the portable abstraction code in a separate pull request. Please focus this on only the provider, i.e., remove `RegionScopedSwiftBlobStore` changes and only add `RegionScopedSwiftBlobStore`. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#issuecomment-300609591
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul commented on this pull request. > +* @param containerAndKey +* @return array of size 2 with container and objectPrefix +*/ + private String[] splitContainerAndKey(String containerAndKey) { + String[] parts = containerAndKey.split("/", 2); + checkArgument(parts.length == 2, "No / separator found in \"%s\"", containerAndKey); + return parts; + } + + /** +* Retrieves a list of existing storage +* @param container +* @param options +* @return maximum of 10,000 object names per request. +*/ + private PageSet list(final String container, `ObjectApi.list(ListContainerOptions)` Javadoc says: > Lists up to 10,000 objects. To control a large list of containers beyond > 10,000 objects, use the `marker` and `endMarker` parameters in the > `ListContainerOptions` class. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r114710757
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul commented on this pull request. > + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jclouds.openstack.swift.v1.options; + +import com.google.common.util.concurrent.ListeningExecutorService; +import org.jclouds.blobstore.options.PutOptions; + +public class OpenStackSwiftPutOptions extends PutOptions { The portable abstraction does not expose all functionality. The caller must use the DLO API to access DLO. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r114710273
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
archupsg03 commented on this pull request. > + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jclouds.openstack.swift.v1.options; + +import com.google.common.util.concurrent.ListeningExecutorService; +import org.jclouds.blobstore.options.PutOptions; + +public class OpenStackSwiftPutOptions extends PutOptions { Say if the user prefers to call putMultipart Blob with DLO, doesn't want to use DLO API. How can such cases be handled ? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r114701786
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
archupsg03 commented on this pull request. > +* @param containerAndKey +* @return array of size 2 with container and objectPrefix +*/ + private String[] splitContainerAndKey(String containerAndKey) { + String[] parts = containerAndKey.split("/", 2); + checkArgument(parts.length == 2, "No / separator found in \"%s\"", containerAndKey); + return parts; + } + + /** +* Retrieves a list of existing storage +* @param container +* @param options +* @return maximum of 10,000 object names per request. +*/ + private PageSet list(final String container, Theres problem in using objectApi.List. I am not able to get the markers. How does objecApi.List handles once it lists the first 10k objects. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r114701694
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul commented on this pull request. > +import org.jclouds.openstack.swift.v1.domain.SwiftObject; +import org.jclouds.openstack.swift.v1.internal.BaseSwiftApiLiveTest; +import org.jclouds.openstack.swift.v1.options.ListContainerOptions; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +import com.google.common.base.Function; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Lists; +import com.google.common.io.ByteSource; +import com.google.common.util.concurrent.Uninterruptibles; + +@Test(groups = "live", testName = "DynamicLargeObjectApiLiveTest", singleThreaded = true) +public class DynamiclargeObjectApiLiveTest extends BaseSwiftApiLiveTest { Rename class to `DynamicLargeObjectApiLiveTest` with a capital "L". -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#pullrequestreview-35918674
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
> For DLO upload, etag will the md5 of the manifest and not the md5 of the > concatenated etags. > Thats the reason etag mismatch happens in ParallelLiveTest. So etag checking > for DLO objects requires > manifest to be constructed locally and compute its md5 and then compare with > the etag from Swift We cannot introduce failing tests so how do you plan to address this? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#issuecomment-298792803
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul commented on this pull request. > + assertReplaceManifest(regionId, defaultContainerName, defaultName); + } + } + + protected void assertReplaceManifest(String regionId, String containerName, String name) { + ObjectApi objectApi = getApi().getObjectApi(regionId, containerName); + + String etag1s = objectApi.put(name + "/1", newByteSourcePayload(ByteSource.wrap(megOf1s))); + awaitConsistency(); + assertMegabyteAndETagMatches(regionId, containerName, name + "/1", etag1s); + + String etag2s = objectApi.put(name + "/2", newByteSourcePayload(ByteSource.wrap(megOf2s))); + awaitConsistency(); + assertMegabyteAndETagMatches(regionId, containerName, name + "/2", etag2s); + + List segments = ImmutableList. builder() If the inconsistency bothers you, please submit a fix for the existing SLO tests as well. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r114451267
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul commented on this pull request. > + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jclouds.openstack.swift.v1.options; + +import com.google.common.util.concurrent.ListeningExecutorService; +import org.jclouds.blobstore.options.PutOptions; + +public class OpenStackSwiftPutOptions extends PutOptions { Once you address https://github.com/jclouds/jclouds/pull/1090#discussion_r114451055 this code becomes unnecessary. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r114451195
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul commented on this pull request. >return api.getStaticLargeObjectApi(regionId, > mpu.containerName()).replaceManifest(mpu.blobName(), builder.build(), mpu.blobMetadata().getUserMetadata(), getContentMetadataForManifest(mpu.blobMetadata().getContentMetadata())); } + + public String completeMultipartUploadDLO(MultipartUpload mpu, List parts) { jclouds consists of two levels, the lower-level provider APIs, e.g., `DynamicLargeObjectApi`, and the higher-level portable abstraction, e.g., `RegionScopedSwiftBlobStore`. The former exposes all the functionality of a given provider, e.g, SLO, DLO, etc., while the latter only implements a portable layer which is common between providers. Thus `completeMultipartUploadDLO` should not exist and if callers want this functionality they can use `RegionScopedSwiftBlobStore`. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r114451055
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
archupsg03 commented on this pull request. >return api.getStaticLargeObjectApi(regionId, > mpu.containerName()).replaceManifest(mpu.blobName(), builder.build(), mpu.blobMetadata().getUserMetadata(), getContentMetadataForManifest(mpu.blobMetadata().getContentMetadata())); } + + public String completeMultipartUploadDLO(MultipartUpload mpu, List parts) { Not getting this point too -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r114284043
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
archupsg03 commented on this pull request. > + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jclouds.openstack.swift.v1.options; + +import com.google.common.util.concurrent.ListeningExecutorService; +import org.jclouds.blobstore.options.PutOptions; + +public class OpenStackSwiftPutOptions extends PutOptions { Am not getting this point :( -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r114283976
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
For DLO upload, etag will the md5 of the manifest and not the content of the file thats getting uploaded. Thats the reason etag mismatch happens in ParallelLiveTest -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#issuecomment-298577286
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
archupsg03 commented on this pull request. >} - return api.getContainerApi(regionId).create(container, BASIC_CONTAINER); + resultContainerCreate = api.getContainerApi(regionId).create(container, BASIC_CONTAINER); + if (resultContainerCreate) { + Container val = api.getContainerApi(regionId).get(container); + containerCache.put(container, Optional.fromNullable(val)); + } + return resultContainerCreate; This is a separate fix. Will submit another PR -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r114249131
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
archupsg03 commented on this pull request. > + assertReplaceManifest(regionId, defaultContainerName, defaultName); + } + } + + protected void assertReplaceManifest(String regionId, String containerName, String name) { + ObjectApi objectApi = getApi().getObjectApi(regionId, containerName); + + String etag1s = objectApi.put(name + "/1", newByteSourcePayload(ByteSource.wrap(megOf1s))); + awaitConsistency(); + assertMegabyteAndETagMatches(regionId, containerName, name + "/1", etag1s); + + String etag2s = objectApi.put(name + "/2", newByteSourcePayload(ByteSource.wrap(megOf2s))); + awaitConsistency(); + assertMegabyteAndETagMatches(regionId, containerName, name + "/2", etag2s); + + List segments = ImmutableList. builder() These are just copied from SLO testcase, and appropriate changes has been made for DLO. These things remain the same. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r114249058
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
archupsg03 commented on this pull request. > @@ -123,6 +124,50 @@ public void uploadMultipartBlob() { // The etag returned by Swift is not the md5 of the Blob uploaded // It is the md5 of the concatenated segment md5s } + + @Test + public void uploadMultipartBlobDLO() { + Blob blob = blobStore.blobBuilder(bigFile.getName()) +.payload(new FilePayload(bigFile)) +.build(); + // configure the blobstore to use multipart uploading of the file + OpenStackSwiftPutOptions opt = new OpenStackSwiftPutOptions(); + opt.multipart(executor); + opt.DLO(true); + String eTag = blobStore.putBlob(CONTAINER, blob, opt); + // assertEquals(eTag, etag); + // The etag returned by Swift is not the md5 of the Blob uploaded + // It is the md5 of the concatenated segment md5s Etag comparison was already commented in the previous function -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r114249006
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
archupsg03 commented on this pull request. > +* @param containerAndKey +* @return array of size 2 with container and objectPrefix +*/ + private String[] splitContainerAndKey(String containerAndKey) { + String[] parts = containerAndKey.split("/", 2); + checkArgument(parts.length == 2, "No / separator found in \"%s\"", containerAndKey); + return parts; + } + + /** +* Retrieves a list of existing storage +* @param container +* @param options +* @return maximum of 10,000 object names per request. +*/ + private PageSet list(final String container, Deleting DLO objects requires, the details of the segments. BlobStore's List command takes BlobStore's ListContainerOptions as input. But incase of remove blob we need Swift's ListContainerOptions. That the reasonf i copied and made few changes -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#discussion_r114248882
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul commented on this pull request. > +*/ + private void removeObjectsWithPrefix(String containerAndPrefix) { + String[] parts = splitContainerAndKey(containerAndPrefix); + + String container = parts[0]; + String prefix = parts[1]; + + removeObjectsWithPrefix(container, prefix); + } + + + /** +* @param containerAndKey +* @return array of size 2 with container and objectPrefix +*/ + private String[] splitContainerAndKey(String containerAndKey) { Add `static`. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#pullrequestreview-35522755
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul commented on this pull request. > +* @param prefix +* +*/ + private void removeObjectsWithPrefix(String container, String prefix) { + String nextMarker = null; + ObjectApi objectApi = api.getObjectApi(regionId, container); + do { + org.jclouds.openstack.swift.v1.options.ListContainerOptions listContainerOptions = prefix(prefix); + if (nextMarker != null) { +listContainerOptions = listContainerOptions.marker(nextMarker); + } + PageSet chunks = list(container, listContainerOptions); + Iterator iter = chunks.iterator(); + + while (iter.hasNext()) { +objectApi.delete(iter.next().getName()); Call `BulkApi.buckDelete`. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#pullrequestreview-35522686
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul commented on this pull request. > +*/ + private String getSLOInfo(String container, String objectName) { + ObjectApi objectApi = api.getObjectApi(regionId, container); + MultimapobjInfo = objectApi.getWithoutBody(objectName).getHeaders(); + if (objInfo.containsKey("X-Static-Large-Object")) { + Collection objectManifest = objInfo.get("X-Static-Large-Object"); + return String.valueOf(objectManifest.toArray()[0]); + } + return null; + } + + + /** +* @param containerAndPrefix +*/ + private void removeObjectsWithPrefix(String containerAndPrefix) { Remove this function and make the single caller explicitly call `splitContainerAndKey` and `removeObjectsWithPrefix(String, String)`. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#pullrequestreview-35522625
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul commented on this pull request. > + > assertEquals(object1s.getPayload().getContentMetadata().getContentLength(), > Long.valueOf(1024 * 1024)); + } + + protected void deleteAllObjectsInContainerDLO(String regionId, final String containerName) { + Uninterruptibles.sleepUninterruptibly(10, TimeUnit.SECONDS); + + ObjectList objects = getApi().getObjectApi(regionId, containerName).list(new ListContainerOptions()); + if (objects == null) { + return; + } + List pathsToDelete = Lists.transform(objects, new Function() { + public String apply(SwiftObject input) { +return containerName + "/" + input.getName(); + } + }); + if (!pathsToDelete.isEmpty()) { Unneeded conditional. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#pullrequestreview-35507370
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul commented on this pull request. > + assertReplaceManifest(regionId, defaultContainerName, defaultName); + } + } + + protected void assertReplaceManifest(String regionId, String containerName, String name) { + ObjectApi objectApi = getApi().getObjectApi(regionId, containerName); + + String etag1s = objectApi.put(name + "/1", newByteSourcePayload(ByteSource.wrap(megOf1s))); + awaitConsistency(); + assertMegabyteAndETagMatches(regionId, containerName, name + "/1", etag1s); + + String etag2s = objectApi.put(name + "/2", newByteSourcePayload(ByteSource.wrap(megOf2s))); + awaitConsistency(); + assertMegabyteAndETagMatches(regionId, containerName, name + "/2", etag2s); + + List segments = ImmutableList. builder() Prefer `ImmutableList.of`. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#pullrequestreview-35507367
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul commented on this pull request. > @@ -123,6 +124,50 @@ public void uploadMultipartBlob() { // The etag returned by Swift is not the md5 of the Blob uploaded // It is the md5 of the concatenated segment md5s } + + @Test + public void uploadMultipartBlobDLO() { + Blob blob = blobStore.blobBuilder(bigFile.getName()) +.payload(new FilePayload(bigFile)) +.build(); + // configure the blobstore to use multipart uploading of the file + OpenStackSwiftPutOptions opt = new OpenStackSwiftPutOptions(); + opt.multipart(executor); + opt.DLO(true); + String eTag = blobStore.putBlob(CONTAINER, blob, opt); + // assertEquals(eTag, etag); + // The etag returned by Swift is not the md5 of the Blob uploaded + // It is the md5 of the concatenated segment md5s Why not verify the ETag? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#pullrequestreview-35507363
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul commented on this pull request. > +* @param segments +* ordered parts which will be concatenated upon download. +* @param metadata +* corresponds to {@link SwiftObject#getMetadata()}. +* @param headers +* Binds the map to headers, without prefixing/escaping the header +* name/key. +* +* @return {@link SwiftObject#getEtag()} of the object, which is the MD5 +* checksum of the concatenated ETag values of the {@code segments}. +*/ + @Named("dynamicLargeObject:replaceManifest") + @PUT + @ResponseParser(ETagHeader.class) + @Headers(keys = "X-Object-Manifest", values = "{containerName}/{objectName}/") + String replaceManifest(@PathParam("containerName") String containerName, @PathParam("objectName") String objectName, Annotate this method @Deprecated and document that users should prefer SLO. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#pullrequestreview-35507200
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul commented on this pull request. > return new > PageSetImpl(ImmutableList. of(), null); } else { - containerCache.put(container, Optional.of(objects.getContainer())); + //containerCache.put(container, Optional.of(objects.getContainer())); Why comment this out? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1090#pullrequestreview-35507189
Re: [jclouds/jclouds] JClouds 1281- DLO Swift Implementation (#1090)
andrewgaul requested changes on this pull request. Please address substantial comments and remove all stylistic changes. Also investigate these failed tests: ``` CloudFilesUSBlobIntegrationLiveTest>BaseBlobIntegrationTest.deleteObjectNoContainer:525 » NullPointer CloudFilesUSBlobIntegrationLiveTest>BaseBlobIntegrationTest.deleteObjectNotFound:466 » NullPointer CloudFilesUSBlobIntegrationParallelLiveTest>RegionScopedSwiftBlobStoreParallelLiveTest.downloadParallelBlob:177 expected [bf9fdd871a0ba46a93f970aed60d9058] but found [813d97604fbcd3961dd2be9a99f9ed41] CloudFilesUSBlobIntegrationParallelLiveTest>RegionScopedSwiftBlobStoreParallelLiveTest.streamParallelBlob:194 expected [bf9fdd871a0ba46a93f970aed60d9058] but found [813d97604fbcd3961dd2be9a99f9ed41] ``` >} - return api.getContainerApi(regionId).create(container, BASIC_CONTAINER); + resultContainerCreate = api.getContainerApi(regionId).create(container, BASIC_CONTAINER); + if (resultContainerCreate) { + Container val = api.getContainerApi(regionId).get(container); + containerCache.put(container, Optional.fromNullable(val)); + } + return resultContainerCreate; How is creating a container related to dynamic large objects? If this is a separate change, submit a separate pull request. > @@ -17,6 +17,7 @@ package org.jclouds.openstack.swift.v1; import java.io.Closeable; + Please sweep through the entire pull request and remove these whitespace changes. >// deleting a object only deletes the manifest, leaving the subobjects. // We first try a multipart delete and if that fails since the object is // not an MPU we fall back to single-part delete. - DeleteStaticLargeObjectResponse response = api.getStaticLargeObjectApi(regionId, container).delete(name); - if (!response.status().equals("200 OK")) { + + String objManifest = getManifestInfo(container, name); + String sloInfo = getSLOInfo(container, name); Previously this code issued one or two DELETEs depending if the name was an SLO or a regular object. With this commit we issue always issue three RPCs, getWithoutBody, getWithoutBody, and delete with some flags. How can you optimize this? Also this fails if the object does not exist. >return api.getStaticLargeObjectApi(regionId, > mpu.containerName()).replaceManifest(mpu.blobName(), builder.build(), mpu.blobMetadata().getUserMetadata(), getContentMetadataForManifest(mpu.blobMetadata().getContentMetadata())); } + + public String completeMultipartUploadDLO(MultipartUpload mpu, List parts) { Remove - we should not expose dynamic large objects in the portable abstraction! Users can call the provider-level APIs for this functionality. > + } + return null; + } + + /** +* Gets X-Static-Large-Object for SLO uploaded file +* @param container +* @param objectName +* @return true if object is SLO uploaded else null +*/ + private String getSLOInfo(String container, String objectName) { + ObjectApi objectApi = api.getObjectApi(regionId, container); + MultimapobjInfo = objectApi.getWithoutBody(objectName).getHeaders(); + if (objInfo.containsKey("X-Static-Large-Object")) { + Collection objectManifest = objInfo.get("X-Static-Large-Object"); + return String.valueOf(objectManifest.toArray()[0]); Consider the simpler: ```java ObjectApi objectApi = api.getObjectApi(regionId, container); Multimap objInfo = objectApi.getWithoutBody(objectName).getHeaders(); Collection objectManifest = objInfo.get("X-Static-Large-Object"); return Iterables.getFirst(objectManifest, null); ``` > +*/ + private String getSLOInfo(String container, String objectName) { + ObjectApi objectApi = api.getObjectApi(regionId, container); + Multimap objInfo = objectApi.getWithoutBody(objectName).getHeaders(); + if (objInfo.containsKey("X-Static-Large-Object")) { + Collection objectManifest = objInfo.get("X-Static-Large-Object"); + return String.valueOf(objectManifest.toArray()[0]); + } + return null; + } + + + /** +* @param containerAndPrefix +*/ + private void removeObjectsWithPrefix(String containerAndPrefix) { Add `static`. > +* @param containerAndKey +* @return array of size 2 with container and objectPrefix +*/ + private String[] splitContainerAndKey(String containerAndKey) { + String[] parts = containerAndKey.split("/", 2); + checkArgument(parts.length == 2, "No / separator found in \"%s\"", containerAndKey); + return parts; + } + + /** +* Retrieves a list of existing storage +* @param container +* @param options +* @return maximum of 10,000 object names per request. +*/ + private PageSet list(final String container, Why do you reimplement `ObjectApi.list`? > + * the License. You may