You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by danbroudy <no...@github.com> on 2015/06/09 19:27:54 UTC
[jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
This change will make implementing the blobstore multipart uploads using simple uploads and compose much cleaner.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs-google/pull/150
-- Commit Summary --
* Update ComposeObjectTemplate and update MockTests
-- File Changes --
M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/strategy/internal/SequentialMultipartUploadStrategy.java (4)
M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/templates/ComposeObjectTemplate.java (85)
M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/features/ObjectApiLiveTest.java (11)
M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/features/ObjectApiMockTest.java (10)
M google-cloud-storage/src/test/resources/object_compose_request.json (45)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/internal/BaseGoogleComputeEngineApiLiveTest.java (2)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-google/pull/150.patch
https://github.com/jclouds/jclouds-labs-google/pull/150.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by Andrew Gaul <no...@github.com>.
> + }
> + this.sourceObjects = ImmutableList.copyOf(sourceObjects);
> + return this;
> + }
> +
> + public Builder fromNames(List<String> SourceObjectNames){
> + ArrayList<SourceObject> sourceObjects = new ArrayList<SourceObject>();
> + for (String name : SourceObjectNames){
> + sourceObjects.add(SourceObject.nameOnly(name));
> + }
> + this.sourceObjects = ImmutableList.copyOf(sourceObjects);
> + return this;
> + }
> +
> + public Builder destination(ObjectTemplate destination){
> + this.destination = destination;
Check for null?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32684902
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by danbroudy <no...@github.com>.
> return new AutoValue_ComposeObjectTemplate(sourceObjects, destination);
> }
> +
> + ComposeObjectTemplate() {
> + }
> +
> + public static class Builder {
> + private ImmutableList<SourceObject> sourceObjects;
> + private ObjectTemplate destination;
> +
> + public Builder(){
> + }
> +
> + public Builder fromGoogleCloudStorageObject(List<GoogleCloudStorageObject> objects){
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32880378
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by danbroudy <no...@github.com>.
> @@ -44,7 +44,7 @@
> public class BaseGoogleComputeEngineApiLiveTest extends BaseApiLiveTest<GoogleComputeEngineApi> {
>
> protected static final String ZONE_API_URL_SUFFIX = "/zones/";
> - protected static final String DEFAULT_ZONE_NAME = "us-central1-a";
> + protected static final String DEFAULT_ZONE_NAME = "us-central1-f";
Yes, us-central1-a gets a lot of attention, its nice to mix it up.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32879925
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by danbroudy <no...@github.com>.
ping
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150#issuecomment-112969739
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by Andrew Gaul <no...@github.com>.
Pushed to master as 45c6c36734c8f783ddeed5e08f48f1f5e77cda0f and 1.9.x as ab9be7bf24b9f80e71f97592a1c1b86de9f72d5f.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150#issuecomment-114205995
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by Andrew Gaul <no...@github.com>.
> return new AutoValue_ComposeObjectTemplate(sourceObjects, destination);
> }
> +
> + ComposeObjectTemplate() {
> + }
> +
> + public static class Builder {
> + private ImmutableList<SourceObject> sourceObjects;
> + private ObjectTemplate destination;
> +
> + public Builder(){
> + }
> +
> + public Builder fromGoogleCloudStorageObject(List<GoogleCloudStorageObject> objects){
> + ArrayList<SourceObject> sourceObjects = new ArrayList<SourceObject>();
> + for (GoogleCloudStorageObject obj : objects){
Space before brace?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32684927
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by Andrew Gaul <no...@github.com>.
> return new AutoValue_ComposeObjectTemplate(sourceObjects, destination);
> }
> +
> + ComposeObjectTemplate() {
> + }
> +
> + public static class Builder {
> + private ImmutableList<SourceObject> sourceObjects;
> + private ObjectTemplate destination;
> +
> + public Builder(){
> + }
> +
> + public Builder fromGoogleCloudStorageObject(List<GoogleCloudStorageObject> objects){
> + ArrayList<SourceObject> sourceObjects = new ArrayList<SourceObject>();
Why not use `ImmutableList.builder`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32684876
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by danbroudy <no...@github.com>.
> return new AutoValue_ComposeObjectTemplate(sourceObjects, destination);
> }
> +
> + ComposeObjectTemplate() {
> + }
> +
> + public static class Builder {
> + private ImmutableList<SourceObject> sourceObjects;
> + private ObjectTemplate destination;
> +
> + public Builder(){
> + }
I don't doubt there is a better way. Im not sure what you are envisioning, are you suggesting I put a static `builder` method be in the ComposeObjectTemplate class and not ComposeObjectTemplate.Builder class and make the current `public Builder()` method package protected?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32880446
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by danbroudy <no...@github.com>.
> return new AutoValue_ComposeObjectTemplate(sourceObjects, destination);
> }
> +
> + ComposeObjectTemplate() {
> + }
> +
> + public static class Builder {
> + private ImmutableList<SourceObject> sourceObjects;
> + private ObjectTemplate destination;
> +
> + public Builder(){
> + }
> +
> + public Builder fromGoogleCloudStorageObject(List<GoogleCloudStorageObject> objects){
> + ArrayList<SourceObject> sourceObjects = new ArrayList<SourceObject>();
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32880374
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by Andrew Gaul <no...@github.com>.
> return new AutoValue_ComposeObjectTemplate(sourceObjects, destination);
> }
> +
> + ComposeObjectTemplate() {
> + }
> +
> + public static class Builder {
> + private ImmutableList<SourceObject> sourceObjects;
> + private ObjectTemplate destination;
> +
> + public Builder() {
> I don't doubt there is a better way. Im not sure what you are envisioning, are you suggesting I put a static `builder` method be in the ComposeObjectTemplate class and not ComposeObjectTemplate.Builder class and make the current `public Builder()` method package protected?
Correct; we should prefer a private constructor and public static `builder` method which calls it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32880531
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by danbroudy <no...@github.com>.
> + }
> + this.sourceObjects = ImmutableList.copyOf(sourceObjects);
> + return this;
> + }
> +
> + public Builder fromNames(List<String> SourceObjectNames){
> + ArrayList<SourceObject> sourceObjects = new ArrayList<SourceObject>();
> + for (String name : SourceObjectNames){
> + sourceObjects.add(SourceObject.nameOnly(name));
> + }
> + this.sourceObjects = ImmutableList.copyOf(sourceObjects);
> + return this;
> + }
> +
> + public Builder destination(ObjectTemplate destination){
> + this.destination = destination;
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32880367
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by danbroudy <no...@github.com>.
> return new AutoValue_ComposeObjectTemplate(sourceObjects, destination);
> }
> +
> + ComposeObjectTemplate() {
> + }
> +
> + public static class Builder {
> + private ImmutableList<SourceObject> sourceObjects;
> + private ObjectTemplate destination;
> +
> + public Builder(){
> + }
> +
> + public Builder fromGoogleCloudStorageObject(List<GoogleCloudStorageObject> objects){
> + ArrayList<SourceObject> sourceObjects = new ArrayList<SourceObject>();
> + for (GoogleCloudStorageObject obj : objects){
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32880366
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by Andrew Gaul <no...@github.com>.
> return new AutoValue_ComposeObjectTemplate(sourceObjects, destination);
> }
> +
> + ComposeObjectTemplate() {
> + }
> +
> + public static class Builder {
> + private ImmutableList<SourceObject> sourceObjects;
> + private ObjectTemplate destination;
> +
> + public Builder(){
> + }
> +
> + public Builder fromGoogleCloudStorageObject(List<GoogleCloudStorageObject> objects){
Take a `Collection`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32684852
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by Andrew Gaul <no...@github.com>.
> @@ -44,7 +44,7 @@
> public class BaseGoogleComputeEngineApiLiveTest extends BaseApiLiveTest<GoogleComputeEngineApi> {
>
> protected static final String ZONE_API_URL_SUFFIX = "/zones/";
> - protected static final String DEFAULT_ZONE_NAME = "us-central1-a";
> + protected static final String DEFAULT_ZONE_NAME = "us-central1-f";
Intentional?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32684704
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by danbroudy <no...@github.com>.
> return new AutoValue_ComposeObjectTemplate(sourceObjects, destination);
> }
> +
> + ComposeObjectTemplate() {
> + }
> +
> + public static class Builder {
> + private ImmutableList<SourceObject> sourceObjects;
> + private ObjectTemplate destination;
> +
> + public Builder() {
Sounds good to me, updated.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32907151
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by Andrew Gaul <no...@github.com>.
> return new AutoValue_ComposeObjectTemplate(sourceObjects, destination);
> }
> +
> + ComposeObjectTemplate() {
> + }
> +
> + public static class Builder {
> + private ImmutableList<SourceObject> sourceObjects;
> + private ObjectTemplate destination;
> +
> + public Builder(){
> + }
Should this be package protected and have a `builder` static method?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32684663
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update
MockTests (#150)
Posted by Andrew Gaul <no...@github.com>.
Overall looks good but please address nits.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150#issuecomment-112976334