You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Jeremy Daggett <no...@github.com> on 2014/03/13 21:16:42 UTC

[jclouds-examples] Remove RestContext from Rackspace examples (#35)

This PR removes RestContext from the Rackspace examples as the first step towards the work to be done in [JCLOUDS-346](https://issues.apache.org/jira/browse/JCLOUDS-346).

Please refer to [JCLOUDS-152](https://issues.apache.org/jira/browse/JCLOUDS-152) which discusses the removal of RestContext.

Each updated example was debugged/tested against the Rackspace Cloud IAD region.

The logo link has also been updated post site-branding... Thx!
You can merge this Pull Request by running:

  git pull https://github.com/rackerlabs/jclouds-examples cloudfiles-updates

Or you can view, comment on it, or merge it online at:

  https://github.com/jclouds/jclouds-examples/pull/35

-- Commit Summary --

  * Removed all references to RestContext from Rackspace examples.

-- File Changes --

    M README.md (2)
    M rackspace/src/main/java/org/jclouds/examples/rackspace/Authentication.java (27)
    M rackspace/src/main/java/org/jclouds/examples/rackspace/Logging.java (31)
    M rackspace/src/main/java/org/jclouds/examples/rackspace/cloudblockstorage/CreateVolumeAndAttach.java (56)
    M rackspace/src/main/java/org/jclouds/examples/rackspace/cloudblockstorage/DetachVolume.java (43)
    M rackspace/src/main/java/org/jclouds/examples/rackspace/cloudblockstorage/ListVolumeAttachments.java (36)
    M rackspace/src/main/java/org/jclouds/examples/rackspace/cloudfiles/Constants.java (3)
    M rackspace/src/main/java/org/jclouds/examples/rackspace/cloudfiles/CreateContainer.java (33)
    M rackspace/src/main/java/org/jclouds/examples/rackspace/cloudfiles/CrossOriginResourceSharingContainer.java (42)
    M rackspace/src/main/java/org/jclouds/examples/rackspace/cloudfiles/DeleteObjectsAndContainer.java (40)
    M rackspace/src/main/java/org/jclouds/examples/rackspace/cloudfiles/ListContainers.java (30)
    M rackspace/src/main/java/org/jclouds/examples/rackspace/cloudfiles/ListObjects.java (36)
    M rackspace/src/main/java/org/jclouds/examples/rackspace/cloudfiles/UploadDirectoryToCDN.java (39)
    M rackspace/src/main/java/org/jclouds/examples/rackspace/cloudfiles/UploadObjects.java (42)
    M rackspace/src/main/java/org/jclouds/examples/rackspace/cloudservers/CreateServerWithKeyPair.java (64)
    M rackspace/src/main/java/org/jclouds/examples/rackspace/cloudservers/ServerMetadata.java (39)

-- Patch Links --

https://github.com/jclouds/jclouds-examples/pull/35.patch
https://github.com/jclouds/jclouds-examples/pull/35.diff

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Ignasi Barrera <no...@github.com>.
[Merged](https://git-wip-us.apache.org/repos/asf?p=jclouds-examples.git;a=commit;h=40da9b47850f6794381f9cd7557ab034300aa375). Thanks!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35#issuecomment-38190676

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Andrew Phillips <no...@github.com>.
> @@ -22,12 +22,13 @@
>   * Constants used by the Rackspace Examples.
>   * 
>   * @author Everett Toews
> + * @authot Jeremy Daggett

[minor] "author"

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35/files#r10816711

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Andrew Phillips <no...@github.com>.
Most of the cleanup is great, thanks @jdaggett! Just a comment about those examples where we use both the ComputeService view and the API. This refactoring now ends up creating _two_ contexts in those cases, which I'm not sure is a good thing to be recommending?

@everett-toews @zack-shoylev What do you think..?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35#issuecomment-38222173

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Andrew Phillips <no...@github.com>.
>              .credentials(username, apiKey)
> -            .modules(modules)
> -            .buildView(ComputeServiceContext.class);
> -      computeService = context.getComputeService();
> -      nova = context.unwrap();
> -      serverApi = nova.getApi().getServerApiForZone(ZONE);
> -      volumeAttachmentApi = nova.getApi().getVolumeAttachmentExtensionForZone(ZONE).get();
> +            .modules(modules);
> +      computeService = builder.buildView(ComputeServiceContext.class).getComputeService();
> +      nova = builder.buildApi(NovaApi.class);

Same comment as above. Stick to building one context?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35/files#r10816697

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Andrew Phillips <no...@github.com>.
> @@ -77,12 +81,10 @@ public static void main(String[] args) throws IOException {
>     }
>  
>     public UploadDirectoryToCDN(String username, String apiKey) {
> -      BlobStoreContext context = ContextBuilder.newBuilder(PROVIDER)
> -            .credentials(username, apiKey)
> -            .buildView(BlobStoreContext.class);
> -      blobStore = context.getBlobStore();
> -      // can use context.unwrapApi(CloudFilesClient.class) in jclouds 1.7
> -      cloudFilesClient = context.unwrap(CloudFilesApiMetadata.CONTEXT_TOKEN).getApi();
> +      ContextBuilder builder = ContextBuilder.newBuilder(PROVIDER)
> +            .credentials(username, apiKey);
> +      blobStore = builder.buildView(BlobStoreContext.class).getBlobStore();
> +      cloudFilesClient = builder.buildApi(CloudFilesClient.class);

Same as above, again - two contexts created here.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35/files#r10816761

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Jeremy Daggett <no...@github.com>.
@demobox @nacx I agree that we should have a single context. Unfortunately, we can't remove RestContext<S, A> from Nova until the 1.8 timeframe, which is going to be some work. I will submit another PR to revert the changes to the Cloud Servers examples for the time being. Sound good?

cc @everett-toews

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35#issuecomment-38945460

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Ignasi Barrera <no...@github.com>.
Closed #35.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Andrew Phillips <no...@github.com>.
>  I will submit another PR to revert the changes to the Cloud Servers examples for the time being. 
> Sound good?

Sounds good! Thanks, @jdaggett!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35#issuecomment-39008301

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Jeremy Daggett <no...@github.com>.
Yes! I ran SmokeTest against the code that was changed and everything passes.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35#issuecomment-37834611

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Andrew Phillips <no...@github.com>.
>              .credentials(username, apiKey)
>              .overrides(overrides)
> -            .modules(modules)
> -            .buildView(ComputeServiceContext.class);
> -
> -      computeService = context.getComputeService();
> -      novaContext = context.unwrap();
> +            .modules(modules);
> +      computeService = builder.buildView(ComputeServiceContext.class).getComputeService();
> +      nova = builder.buildApi(NovaApi.class);

And here ;-)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35/files#r10816775

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Everett Toews <no...@github.com>.
Did you run the SmokeTest with these changes?

When that passes, I'm a +1.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35#issuecomment-37594984

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Andrew Phillips <no...@github.com>.
> @@ -67,11 +71,10 @@ public static void main(String[] args) throws IOException {
>     }
>  
>     public UploadObjects(String username, String apiKey) {
> -      BlobStoreContext context = ContextBuilder.newBuilder(PROVIDER)
> -            .credentials(username, apiKey)
> -            .buildView(BlobStoreContext.class);
> -      blobStore = context.getBlobStore();
> -      swift = context.unwrap();
> +      ContextBuilder builder = ContextBuilder.newBuilder(PROVIDER)
> +                                  .credentials(username, apiKey);
> +      blobStore = builder.buildView(BlobStoreContext.class).getBlobStore();
> +      swift = builder.buildApi(CloudFilesClient.class);

And again here.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35/files#r10816767

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Ignasi Barrera <no...@github.com>.
Didn't notice that when going through the PR. and agree. Examples should be creating only one context!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35#issuecomment-38228840

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Andrew Phillips <no...@github.com>.
>  I can update with your suggestions in the next PR which is underway now. WDYT?

Works for me! Would just be curious to see what we feel collectively about creating two contexts vs. using `unwrap` to get at the API...

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35#issuecomment-38223009

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Andrew Phillips <no...@github.com>.
>              .credentials(username, apiKey)
>              .modules(modules)
> -            .overrides(overrides)
> -            .buildView(ComputeServiceContext.class);
> -      computeService = context.getComputeService();
> -      nova = context.unwrap();
> -      volumeAttachmentApi = nova.getApi().getVolumeAttachmentExtensionForZone(ZONE).get();
> +            .overrides(overrides);
> +
> +      computeService = builder.buildView(ComputeServiceContext.class).getComputeService();
> +      nova = builder.buildApi(NovaApi.class);

Hm...do we recommend building _two separate_ things here? Isn't that effectively creating two contexts, which is quite expensive? In this case, I'd say the old approach (building one context, then getting the compute service and unwrapping the context) is preferable?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35/files#r10816650

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Jeremy Daggett <no...@github.com>.
@demobox Thanks for the feedback! Removal of RestContext was the first step in a bunch of example changes coming. I can update with your suggestions in the next PR which is underway now. WDYT?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35#issuecomment-38222858

Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)

Posted by Andrew Phillips <no...@github.com>.
> @@ -212,6 +215,7 @@ private void mountVolume(NodeMetadata node) {
>      */
>     public void close() throws IOException {
>        Closeables.close(cinderApi, true);
> +      Closeables.close(nova, true);

If we stick with one context, we wouldn't need this, I guess?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/35/files#r10816673