rackerlabs / developer.rackspace.com

Gen 3 of the developer.rackspace.com Portal
Apache License 2.0
47 stars 59 forks source link

Adding jclouds samples for the Rackspace CDN #1094

Closed ycombinator closed 9 years ago

ycombinator commented 9 years ago

Please DO NOT MERGE until jclouds 1.9.0 is released. To check this, search for jclouds 1.9.0 on Maven Central and make sure the search results are not empty.

This PR supersedes #1057.

Note that jclouds does not (yet?) support the Purge Assets API. Consequently this PR does not include Java samples in:

This missing functionality will be added in a later release. When that happens, we will send in a new PR to add Java samples to the two .rst files listed above.

UPDATE: This PR now also includes changes to non-CDN jclouds code - snippets used in each of the services' quick start guides as well as full examples. This is because jclouds 1.9.0 is required to use CDN and that version introduced some backwards-incompatible changes in other services (for details see comments starting with https://github.com/rackerlabs/developer.rackspace.com/pull/1094#issuecomment-87830845).

zack-shoylev commented 9 years ago

Looks good enough to merge. However the jclouds version used needs to also be updated here for this to work.

ycombinator commented 9 years ago

However the jclouds version used needs to also be updated here for this to work.

Could you point me to exactly where that needs to happen and what the new version should be? I'll add that change to this PR then. Thanks.

ycombinator commented 9 years ago

Per @zack-shoylev the version in https://github.com/rackerlabs/developer.rackspace.com/blob/master/src/site_source/sdks/java/pom.xml#L5 should be changed to 1.9.0. Changing.

zack-shoylev commented 9 years ago

Thanks!

zack-shoylev commented 9 years ago

Note: do not merge until jclouds version 1.9.0 is released. This will be visible on jclouds.apache.org

ycombinator commented 9 years ago

@rgbkrk: jclouds 1.9.0 has now been released. This PR is ready for merge.

etoews commented 9 years ago

This PR is good but bumping the version to <jclouds.version>1.9.0</jclouds.version> means changes in other examples as well. All of the examples in the full-examples will need to be checked against 1.9.0 as that's when the Zone to Region rename happened.

ycombinator commented 9 years ago

@everett-toews thanks for bringing up the effects of changing to 1.9.0. Two questions:

  1. Should going from 1.7.3 to 1.9.0 be backwards-compatible?
  2. What's a good way to know about all the changes in 1.9.0 that would require changes to our d.r.c examples? I looked at https://github.com/jclouds/jclouds/releases/tag/jclouds-1.9.0 but didn't find any release notes there.
etoews commented 9 years ago
  1. Not entirely. :disappointed: There were backwards incompatible changes in openstack-swift.
  2. Here are the release notes. You can see the backwards incompatible stuff in the API Changes of 1.8.1

There were lots of deprecations too so any example code based on 1.9.0 should really use the newer code. The best thing to do is peruse this PR. That should give you a feel for the changes pretty quickly.

ycombinator commented 9 years ago

Just so there's no's ambiguity: I'm going to update this PR to include any code changes to quickstart snippets and full examples due to backwards-incompatible changes in jclouds 1.9.0. Marking PR as WIP now.

ycombinator commented 9 years ago

@everett-toews @zack-shoylev This PR is ready for review/merge.

I've updated all jclouds full examples as well as all jclouds code snippets for all services' quick start guides to be 1.9.0 compatible. I've been able to successfully compile all full examples using the POM included in this PR.

zack-shoylev commented 9 years ago

Thanks @ycombinator, I will review. That is a substantial PR :)

ycombinator commented 9 years ago

Those were some substantial breaking changes in jclouds :)

zack-shoylev commented 9 years ago

Did you try running the compiled examples? In some cases, when compiling without the auto-service dependency, and then running the test, you might get a run-time exception. Specifically Neutron or CDN would exhibit this. This is because the jar has to be in the class-path (if I understand it correctly).

ycombinator commented 9 years ago

Ah, okay. Will try to run now (might take a couple of hours).

zack-shoylev commented 9 years ago

+1

ycombinator commented 9 years ago

Following full examples ran successfully:

* I had to define a new waitForLoadBalancerToBecomeActive method that would block until the load balancer was in ACTIVE status. I call this new method right before createHealthMonitor(), setThrottling(), blacklistIPs(), enableContentCaching(), setCustomErrorPage(), and deleteNodes(). Without this mechanism in place, the HTTP API was returning a 422 response with message = "Load Balancer 'XXXXX' has a status of 'PENDING_UPDATE' and is considered immutable.".

ycombinator commented 9 years ago

@zack-shoylev, @everett-toews I have tested all the full examples successfully (see previous comment). This PR is once again ready for review/merge. Thanks!

etoews commented 9 years ago

I'll give this a review this morning.

etoews commented 9 years ago

My review is done. A hand full of comments but overall looks good!

ycombinator commented 9 years ago

@zack-shoylev @everett-toews I've made all the changes you suggested (I think). Could you pretty please review one more time? Thanks.

etoews commented 9 years ago

:+1:

@ycombinator It was an honour to review this PR. It has been an privilege and pleasure working with you. Good day to you sir.

ycombinator commented 9 years ago

@everett-toews :tophat: tip to you as well sir.

Now what does one have to do around here to get a merge!? :smile:

smashwilson commented 9 years ago

A :+1: from @everett-toews works for me :grin:

shipit

zack-shoylev commented 9 years ago

I am quite happy how this turned out.