Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+1 - looks good to me, pending a squash'n'rebase. Thanks, @jasdeep-hundal! @everett-toews @nacx: Any further questions..? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39293321
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
LGTM too. Thanks @jasdeep-hundal! It's good to see this kind of PRs that also take care of adding more tests! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39295055
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
Could you squash and rebase the commits, @jasdeep-hundal? Then we can finally merge this change. Thanks! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39323732
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
@demobox done! Thanks for the help! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39340891
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
[jclouds » jclouds-labs-openstack #998](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/998/) SUCCESS This pull request looks good [(what's this?)](https://www.cloudbees.com/what-is-buildhive) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39341818
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
[jclouds-labs-openstack-pull-requests #196](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/196/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39341943
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
[jclouds-labs-openstack-pull-requests #197](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/197/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39341990
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
[jclouds » jclouds-labs-openstack #999](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/999/) SUCCESS This pull request looks good [(what's this?)](https://www.cloudbees.com/what-is-buildhive) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39342874
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
Committed to [master](https://git-wip-us.apache.org/repos/asf?p=jclouds-labs-openstack.git;a=commit;h=7c1b681b6699025aa56e6b64c7da9c0136349b00) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39352640
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ throw Throwables.propagate(ex);
+ } catch (IOException ex) {
+ throw Throwables.propagate(ex);
+ }
+ throw new HttpException(Glance endpoint does not support
API version: + apiVersion);
+ }
+ });
+ }
+
+ @Override
+ public URI apply(Object from) {
+ checkArgument(from instanceof String, you must specify a zone, as a
String argument);
+ MapString, SupplierURI zoneToEndpoint =
zoneToEndpointSupplier.get();
+ checkState(!zoneToEndpoint.isEmpty(), no zone name to endpoint
mappings configured!);
+ checkArgument(zoneToEndpoint.containsKey(from),
+ requested location %s, which is not in the configured
locations: %s, from, zoneToEndpoint);
This was copied from ZoneToEndpoint in JClouds core
Ah, OK, then let's keep as-is. Thanks for explaining!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11156760
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+import org.jclouds.http.HttpRequest;
+import org.jclouds.http.HttpResponse;
+import org.jclouds.openstack.glance.v1_0.GlanceApi;
+import org.jclouds.openstack.glance.v1_0.internal.BaseGlanceApiExpectTest;
+import org.jclouds.openstack.glance.v1_0.parse.ParseImagesTest;
+import org.testng.annotations.Test;
+
+import com.google.common.util.concurrent.UncheckedExecutionException;
+
+/**
+ * @author Jasdeep Hundal
+ */
+@Test(groups = unit, testName = GlanceVersionNegotiationExpectTest)
+public class GlanceVersionNegotiationExpectTest extends
BaseGlanceApiExpectTest {
+
+ public void testSchemeMismatch() throws Exception {
Thanks for the update, @jasdeep-hundal!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11156769
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ VersionsJsonResponse versions =
json.fromJson(Strings2.toStringAndClose(response),
VersionsJsonResponse.class);
+ for (VersionsJsonResponse.Version version :
versions.versions) {
+if (apiVersion.equals(version.id)) {
+ URI versionedEndpointUri = new
URI(version.links.get(0).href);
+ return new URI(baseEndpointUri.getScheme(),
versionedEndpointUri.getUserInfo(),
+ versionedEndpointUri.getHost(),
versionedEndpointUri.getPort(),
+ versionedEndpointUri.getPath(),
versionedEndpointUri.getQuery(),
+ versionedEndpointUri.getFragment());
+}
+ }
+ } catch (URISyntaxException ex) {
+ throw Throwables.propagate(ex);
+ } catch (IOException ex) {
+ throw Throwables.propagate(ex);
+ }
+ throw new HttpException(Glance endpoint does not support
API version: + apiVersion);
Thanks for the pointer @nacx . @demobox : From that list I think
UnsupportedOperationException makes the most sense, any opinion on that?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11175623
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ if (!baseEndpointPathParts.isEmpty()
+
baseEndpointPathParts.get(baseEndpointPathParts.size() -
1).matches(v[0-9]+(\\.[0-9])?[0-9]*)){
+baseEndpointUri = new
URI(baseEndpointUri.getScheme(), baseEndpointUri.getUserInfo(),
+ baseEndpointUri.getHost(),
baseEndpointUri.getPort(),
+
Joiner.on('/').join(baseEndpointPathParts.subList(0,
baseEndpointPathParts.size() - 1)) + /,
+ baseEndpointUri.getQuery(),
baseEndpointUri.getFragment());
+ }
+
+ HttpRequest negotiationRequest = HttpRequest.builder()
+.method(GET).endpoint(baseEndpointUri)
+.addHeader(VERSION_NEGOTIATION_HEADER,
true).build();
+ InputStream response =
client.invoke(negotiationRequest).getPayload().openStream();
+ VersionsJsonResponse versions =
json.fromJson(Strings2.toStringAndClose(response),
VersionsJsonResponse.class);
+ for (VersionsJsonResponse.Version version :
versions.versions) {
+if (apiVersion.equals(version.id)) {
+ URI versionedEndpointUri = new
URI(version.links.get(0).href);
Haven't found a proper spec, but from talking to Mark I'm very confident this
is the right thing and have yet to see an endpoint that returns multiple
endpoints for a version.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11175754
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
@demobox @everett-toews @nacx : All comments addressed, thanks again for taking the time to review this. I definitely want to get this right, so keep the comments/concerns coming. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39248122
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
[jclouds-labs-openstack-pull-requests #192](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/192/) UNSTABLE Looks like there's a problem with this pull request --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39248959
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
[jclouds » jclouds-labs-openstack #988](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/988/) UNSTABLE Looks like there's a problem with this pull request [(what's this?)](https://www.cloudbees.com/what-is-buildhive) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39249047
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
Hmm, test failures that I didn't see locally, will look into that soon. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39250229
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
[jclouds-labs-openstack-pull-requests #193](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/193/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39253920
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
Tests fixed after realizing they were running on top of https://github.com/jclouds/jclouds-labs-openstack/pull/84 Checkstyle violations also cleared out. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39255824
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
[jclouds-labs-openstack-pull-requests #194](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/194/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39256561
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ VersionsJsonResponse versions =
json.fromJson(Strings2.toStringAndClose(response),
VersionsJsonResponse.class);
+ for (VersionsJsonResponse.Version version :
versions.versions) {
+if (apiVersion.equals(version.id)) {
+ URI versionedEndpointUri = new
URI(version.links.get(0).href);
+ return new URI(baseEndpointUri.getScheme(),
versionedEndpointUri.getUserInfo(),
+ versionedEndpointUri.getHost(),
versionedEndpointUri.getPort(),
+ versionedEndpointUri.getPath(),
versionedEndpointUri.getQuery(),
+ versionedEndpointUri.getFragment());
+}
+ }
+ } catch (URISyntaxException ex) {
+ throw Throwables.propagate(ex);
+ } catch (IOException ex) {
+ throw Throwables.propagate(ex);
+ }
+ throw new UnsupportedOperationException(Glance endpoint
does not support API version: + apiVersion);
Either this or `IllegalArgumentException`, but UOE is probably better, indeed.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11187283
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+
versionRegex.matcher(baseEndpointPathParts.get(baseEndpointPathParts.size() -
1)).matches()) {
+// Constructs a base URI Glance endpoint by
stripping the version from the received URI
+baseEndpointUri = new
URI(baseEndpointUri.getScheme(), baseEndpointUri.getUserInfo(),
+ baseEndpointUri.getHost(),
baseEndpointUri.getPort(),
+
Joiner.on('/').join(baseEndpointPathParts.subList(0,
baseEndpointPathParts.size() - 1)) + /,
+ baseEndpointUri.getQuery(),
baseEndpointUri.getFragment());
+ }
+
+ HttpRequest negotiationRequest = HttpRequest.builder()
+.method(GET).endpoint(baseEndpointUri)
+.addHeader(VERSION_NEGOTIATION_HEADER,
true).build();
+ InputStream response =
client.invoke(negotiationRequest).getPayload().openStream();
+ VersionsJsonResponse versions =
json.fromJson(Strings2.toStringAndClose(response),
VersionsJsonResponse.class);
+ for (VersionsJsonResponse.Version version :
versions.versions) {
+if (apiVersion.equals(version.id)) {
+ URI versionedEndpointUri = new
URI(version.links.get(0).href);
If we are pretty certain that only one element is returned, use
[`Iterable.getOnlyElement`](http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/collect/Iterables.html#getOnlyElement(java.lang.Iterable\))?
Otherwise, at least add a short comment to indicate that we really only expect
one element here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11187328
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+
versionRegex.matcher(baseEndpointPathParts.get(baseEndpointPathParts.size() -
1)).matches()) {
+// Constructs a base URI Glance endpoint by
stripping the version from the received URI
+baseEndpointUri = new
URI(baseEndpointUri.getScheme(), baseEndpointUri.getUserInfo(),
+ baseEndpointUri.getHost(),
baseEndpointUri.getPort(),
+
Joiner.on('/').join(baseEndpointPathParts.subList(0,
baseEndpointPathParts.size() - 1)) + /,
+ baseEndpointUri.getQuery(),
baseEndpointUri.getFragment());
+ }
+
+ HttpRequest negotiationRequest = HttpRequest.builder()
+.method(GET).endpoint(baseEndpointUri)
+.addHeader(VERSION_NEGOTIATION_HEADER,
true).build();
+ InputStream response =
client.invoke(negotiationRequest).getPayload().openStream();
+ VersionsJsonResponse versions =
json.fromJson(Strings2.toStringAndClose(response),
VersionsJsonResponse.class);
+ for (VersionsJsonResponse.Version version :
versions.versions) {
+if (apiVersion.equals(version.id)) {
+ URI versionedEndpointUri = new
URI(version.links.get(0).href);
Did both :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11190337
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
[jclouds » jclouds-labs-openstack #995](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/995/) SUCCESS This pull request looks good [(what's this?)](https://www.cloudbees.com/what-is-buildhive) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39282157
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
[jclouds-labs-openstack-pull-requests #195](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/195/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39282295
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+
+ public void testNonExistentVersion() throws Exception {
+ // The referenced resource only an endpoint for v999.999 of the
GlanceApi
+ HttpResponse localVersionNegotiationResponse =
HttpResponse.builder().statusCode(300).message(HTTP/1.1 300 Multiple
Choices).payload(
+
payloadFromResourceWithContentType(/glanceVersionResponseVersionUnavailable.json,
application/json)).build();
+
+ GlanceApi apiWhenExist =
requestsSendResponses(keystoneAuthWithUsernameAndPassword,
+responseWithKeystoneAccess, versionNegotiationRequest,
localVersionNegotiationResponse);
+
+ try {
+ apiWhenExist.getImageApiForZone(az-1.region-a.geo-1).list();
+ } catch (UncheckedExecutionException e) {
+ if (e.getCause().getClass() == HttpException.class)
+ return;
+ }
+ fail(Did not throw expected HttpException);
Ah, OK, thanks for explaining. In that case, how about using the
`@Test(expected = ...)` annotation in combination with:
```
try {
...
} catch (Unche... e) {
// extract the actual, expected exception from the UncheckedEE wrapper
throw e.getCause();
}
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11098646
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ public ListVersion versions;
+ }
+
+ private final SupplierMapString, SupplierURI zoneToEndpointSupplier;
+ private final String apiVersion;
+ private final LoadingCacheURI, URI endpointCache;
+
+ @Inject
+ public ZoneToEndpointNegotiateVersion(@Zone SupplierMapString,
SupplierURI zoneToEndpointSupplier,
+ @ApiVersion String apiVersionString, final HttpClient client, final
Json json) {
+ this.zoneToEndpointSupplier = checkNotNull(zoneToEndpointSupplier,
zoneToEndpointSupplier);
+ if (!apiVersionString.startsWith(v)) {
+ this.apiVersion = v + apiVersionString;
+ } else {
+ this.apiVersion = apiVersionString;
+ }
so it needs to be be final
Of course - doh! In that case, we could do something like this:
```
public ZoneToEndpointNegotiateVersion(..., @ApiVersion String apiVersion, ...) {
String versionPrefix = apiVersion.startsWith(v) ? : v; // or write
using if...else, if preferred
this.apiVersion = versionPrefix + apiVersion;
}
```
We shouldn't have to worry about hiding if we use `this.apiVersion` to refer to
the field elsewhere in the constructor. I agree, though, that that could be
confusing. In which case, how about calling the parameter apiVersion and the
field something like apiVersionWithPrefix or so, or call the parameter
rawApiVersion and the field apiVersion, to indicate that there's actually
something different about the two?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11098697
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ VersionsJsonResponse versions =
json.fromJson(Strings2.toStringAndClose(response),
VersionsJsonResponse.class);
+ for (VersionsJsonResponse.Version version :
versions.versions) {
+if (apiVersion.equals(version.id)) {
+ URI versionedEndpointUri = new
URI(version.links.get(0).href);
+ return new URI(baseEndpointUri.getScheme(),
versionedEndpointUri.getUserInfo(),
+ versionedEndpointUri.getHost(),
versionedEndpointUri.getPort(),
+ versionedEndpointUri.getPath(),
versionedEndpointUri.getQuery(),
+ versionedEndpointUri.getFragment());
+}
+ }
+ } catch (URISyntaxException ex) {
+ throw Throwables.propagate(ex);
+ } catch (IOException ex) {
+ throw Throwables.propagate(ex);
+ }
+ throw new HttpException(Glance endpoint does not support
API version: + apiVersion);
Jclouds has a restricted set of [propagatable
exceptions](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/util/Throwables2.java#L131-L140).
Could you change it to one from that list? Otherwise the exception will get
encapsulated.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11100300
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+import com.google.common.base.Supplier;
+import com.google.common.base.Throwables;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+
+/**
+ *
+ * @author Jasdeep Hundal
+ */
+@Singleton
+public class ZoneToEndpointNegotiateVersion implements FunctionObject, URI
{
+
+ public static final String VERSION_NEGOTIATION_HEADER =
Is-Version-Negotiation-Request;
+
+ private static class VersionsJsonResponse{
I'm personally ok with leaving this here. Given that we won't expose version
negotiation and will do it transparently, I think it is ok to leave the domain
object and the client used to perform that request hidden in this class.
This pattern is also used with the paged iterables, for example. In Openstack,
Abiquo and others, we use internal classes to deserialize the pagination
requests (see the
[ParseImages](https://github.com/jclouds/jclouds-labs-openstack/blob/master/openstack-glance/src/main/java/org/jclouds/openstack/glance/v1_0/functions/internal/ParseImages.java#L55-L62)
class in this project for an example) and keep the pagination internals hidden
to the user.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11100358
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
Success! ``` Starting test testCreateUpdateAndDeleteImage(org.jclouds.openstack.glance.v1_0.features.ImageApiLiveTest) [TestNG] Test testCreateUpdateAndDeleteImage(org.jclouds.openstack.glance.v1_0.features.ImageApiLiveTest) succeeded: 1187ms Test suite progress: tests succeeded: 1, failed: 0, skipped: 0. Starting test testList(org.jclouds.openstack.glance.v1_0.features.ImageApiLiveTest) [TestNG] Test testList(org.jclouds.openstack.glance.v1_0.features.ImageApiLiveTest) succeeded: 63ms Test suite progress: tests succeeded: 2, failed: 0, skipped: 0. Starting test testListInDetail(org.jclouds.openstack.glance.v1_0.features.ImageApiLiveTest) [TestNG] Test testListInDetail(org.jclouds.openstack.glance.v1_0.features.ImageApiLiveTest) succeeded: 132ms Test suite progress: tests succeeded: 3, failed: 0, skipped: 0. Starting test testReserveUploadAndDeleteImage(org.jclouds.openstack.glance.v1_0.features.ImageApiLiveTest) [TestNG] Test testReserveUploadAndDeleteImage(org.jclouds.openstack.glance.v1_0.features.ImageApiLiveTest) succeeded: 448ms Test suite progress: tests succeeded: 4, failed: 0, skipped: 0. Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 3.194 sec Results : Tests run: 4, Failures: 0, Errors: 0, Skipped: 0 [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 12.744s [INFO] Finished at: Sat Mar 29 12:49:21 PDT 2014 [INFO] Final Memory: 33M/439M [INFO] ``` --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-39007016
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ throw Throwables.propagate(ex);
+ } catch (IOException ex) {
+ throw Throwables.propagate(ex);
+ }
+ throw new HttpException(Glance endpoint does not support
API version: + apiVersion);
+ }
+ });
+ }
+
+ @Override
+ public URI apply(Object from) {
+ checkArgument(from instanceof String, you must specify a zone, as a
String argument);
+ MapString, SupplierURI zoneToEndpoint =
zoneToEndpointSupplier.get();
+ checkState(!zoneToEndpoint.isEmpty(), no zone name to endpoint
mappings configured!);
+ checkArgument(zoneToEndpoint.containsKey(from),
+ requested location %s, which is not in the configured
locations: %s, from, zoneToEndpoint);
Do we need both of these checks? Yes, one throws an ISE and the other an IAE,
but would e.g. the IAE be enough?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11095379
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ public ListVersion versions;
+ }
+
+ private final SupplierMapString, SupplierURI zoneToEndpointSupplier;
+ private final String apiVersion;
+ private final LoadingCacheURI, URI endpointCache;
+
+ @Inject
+ public ZoneToEndpointNegotiateVersion(@Zone SupplierMapString,
SupplierURI zoneToEndpointSupplier,
+ @ApiVersion String apiVersionString, final HttpClient client, final
Json json) {
+ this.zoneToEndpointSupplier = checkNotNull(zoneToEndpointSupplier,
zoneToEndpointSupplier);
+ if (!apiVersionString.startsWith(v)) {
+ this.apiVersion = v + apiVersionString;
+ } else {
+ this.apiVersion = apiVersionString;
+ }
Name the constructor param `apiVersion` and do something like:
```
this.apiVersion = apiVersion;
if (!apiVersion.startsWith(v)) {
this.apiVersion = v + apiVersion;
}
```
which makes it a little clearer that this is exceptional case handling?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11095387
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ @ApiVersion String apiVersionString, final HttpClient client, final
Json json) {
+ this.zoneToEndpointSupplier = checkNotNull(zoneToEndpointSupplier,
zoneToEndpointSupplier);
+ if (!apiVersionString.startsWith(v)) {
+ this.apiVersion = v + apiVersionString;
+ } else {
+ this.apiVersion = apiVersionString;
+ }
+ this.endpointCache = CacheBuilder.newBuilder()
+ .build(
+new CacheLoaderURI, URI() {
+ public URI load(URI baseEndpointUri) {
+ try {
+ ListString baseEndpointPathParts =
Splitter.on('/').omitEmptyStrings().splitToList(baseEndpointUri.getPath());
+ if (!baseEndpointPathParts.isEmpty()
+
baseEndpointPathParts.get(baseEndpointPathParts.size() -
1).matches(v[0-9]+(\\.[0-9])?[0-9]*)){
+baseEndpointUri = new
URI(baseEndpointUri.getScheme(), baseEndpointUri.getUserInfo(),
Quick comment here as to what this line is doing?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11095390
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ public ZoneToEndpointNegotiateVersion(@Zone SupplierMapString,
SupplierURI zoneToEndpointSupplier,
+ @ApiVersion String apiVersionString, final HttpClient client, final
Json json) {
+ this.zoneToEndpointSupplier = checkNotNull(zoneToEndpointSupplier,
zoneToEndpointSupplier);
+ if (!apiVersionString.startsWith(v)) {
+ this.apiVersion = v + apiVersionString;
+ } else {
+ this.apiVersion = apiVersionString;
+ }
+ this.endpointCache = CacheBuilder.newBuilder()
+ .build(
+new CacheLoaderURI, URI() {
+ public URI load(URI baseEndpointUri) {
+ try {
+ ListString baseEndpointPathParts =
Splitter.on('/').omitEmptyStrings().splitToList(baseEndpointUri.getPath());
+ if (!baseEndpointPathParts.isEmpty()
+
baseEndpointPathParts.get(baseEndpointPathParts.size() -
1).matches(v[0-9]+(\\.[0-9])?[0-9]*)){
Either break _before_ `` or break at `.matches`? Also, since this will be
called a few times, extract the regex out into a constant `Pattern` and use a
Matcher.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11095389
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ if (!baseEndpointPathParts.isEmpty()
+
baseEndpointPathParts.get(baseEndpointPathParts.size() -
1).matches(v[0-9]+(\\.[0-9])?[0-9]*)){
+baseEndpointUri = new
URI(baseEndpointUri.getScheme(), baseEndpointUri.getUserInfo(),
+ baseEndpointUri.getHost(),
baseEndpointUri.getPort(),
+
Joiner.on('/').join(baseEndpointPathParts.subList(0,
baseEndpointPathParts.size() - 1)) + /,
+ baseEndpointUri.getQuery(),
baseEndpointUri.getFragment());
+ }
+
+ HttpRequest negotiationRequest = HttpRequest.builder()
+.method(GET).endpoint(baseEndpointUri)
+.addHeader(VERSION_NEGOTIATION_HEADER,
true).build();
+ InputStream response =
client.invoke(negotiationRequest).getPayload().openStream();
+ VersionsJsonResponse versions =
json.fromJson(Strings2.toStringAndClose(response),
VersionsJsonResponse.class);
+ for (VersionsJsonResponse.Version version :
versions.versions) {
+if (apiVersion.equals(version.id)) {
+ URI versionedEndpointUri = new
URI(version.links.get(0).href);
I'm guessing we know this is the correct one simply from the spec?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11095391
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ VersionsJsonResponse versions =
json.fromJson(Strings2.toStringAndClose(response),
VersionsJsonResponse.class);
+ for (VersionsJsonResponse.Version version :
versions.versions) {
+if (apiVersion.equals(version.id)) {
+ URI versionedEndpointUri = new
URI(version.links.get(0).href);
+ return new URI(baseEndpointUri.getScheme(),
versionedEndpointUri.getUserInfo(),
+ versionedEndpointUri.getHost(),
versionedEndpointUri.getPort(),
+ versionedEndpointUri.getPath(),
versionedEndpointUri.getQuery(),
+ versionedEndpointUri.getFragment());
+}
+ }
+ } catch (URISyntaxException ex) {
+ throw Throwables.propagate(ex);
+ } catch (IOException ex) {
+ throw Throwables.propagate(ex);
+ }
+ throw new HttpException(Glance endpoint does not support
API version: + apiVersion);
Is `HttpException` the best choice here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11095394
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
*/
@Test(groups = unit, testName = GlanceErrorHandlerTest)
public class GlanceErrorHandlerTest {
+ @Test
+ public void test300UnintendedVersionNegotiation() {
+ assertCodeMakes(GET, URI
+ .create(https://glance.jclouds.org:9292/;), 300, Multiple
Choices,
+ , HttpResponseException.class);
[minor] Odd line breaks here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11095411
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
HttpResponse response =
HttpResponse.builder().statusCode(statusCode).message(message).payload(content).build();
response.getPayload().getContentMetadata().setContentType(contentType);
expect(command.getCurrentRequest()).andReturn(request).atLeastOnce();
- command.setException(classEq(expected));
+ if (expected != null)
+ command.setException(classEq(expected));
[minor] Use
```
if (...) {
...
}
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11095418
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+
+ public void testNonExistentVersion() throws Exception {
+ // The referenced resource only an endpoint for v999.999 of the
GlanceApi
+ HttpResponse localVersionNegotiationResponse =
HttpResponse.builder().statusCode(300).message(HTTP/1.1 300 Multiple
Choices).payload(
+
payloadFromResourceWithContentType(/glanceVersionResponseVersionUnavailable.json,
application/json)).build();
+
+ GlanceApi apiWhenExist =
requestsSendResponses(keystoneAuthWithUsernameAndPassword,
+responseWithKeystoneAccess, versionNegotiationRequest,
localVersionNegotiationResponse);
+
+ try {
+ apiWhenExist.getImageApiForZone(az-1.region-a.geo-1).list();
+ } catch (UncheckedExecutionException e) {
+ if (e.getCause().getClass() == HttpException.class)
+ return;
+ }
+ fail(Did not throw expected HttpException);
That's what I tried initially, but the exception gets wrapped in an
UncheckedExecutionException and it seemed unclear to use that as the expected
exception in the Test annotation.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11096551
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ public ListVersion versions;
+ }
+
+ private final SupplierMapString, SupplierURI zoneToEndpointSupplier;
+ private final String apiVersion;
+ private final LoadingCacheURI, URI endpointCache;
+
+ @Inject
+ public ZoneToEndpointNegotiateVersion(@Zone SupplierMapString,
SupplierURI zoneToEndpointSupplier,
+ @ApiVersion String apiVersionString, final HttpClient client, final
Json json) {
+ this.zoneToEndpointSupplier = checkNotNull(zoneToEndpointSupplier,
zoneToEndpointSupplier);
+ if (!apiVersionString.startsWith(v)) {
+ this.apiVersion = v + apiVersionString;
+ } else {
+ this.apiVersion = apiVersionString;
+ }
The issue with that is the apiVersion gets used in the CacheLoader, so it needs
to be be final. Since the parameter passed in may need to be modified, I
couldn't give it the same name since it would hide the apiVersion field.
Suggestions to work around this and make it cleaner?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11096564
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
@nacx @demobox @everett-toews : If you folks have time to give this a (hopefully) final review, would love to get this off my plate. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-38977210
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
@jasdeep-hundal Have you tested this against a DevStack instance that's running both versions of the Glance API? Can you please share your DevStack localrc in a gist? Can you please share the jclouds code in a gist that you ran against a DevStack instance? I'd like to try this in a realish env to help me understand it better. Thanks. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-38492402
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
[jclouds » jclouds-labs-openstack #942](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/942/) SUCCESS This pull request looks good [(what's this?)](https://www.cloudbees.com/what-is-buildhive) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-38371981
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
[jclouds-labs-openstack-pull-requests #184](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/184/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-38372033
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
[jclouds » jclouds-labs-openstack #939](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/939/) FAILURE Looks like there's a problem with this pull request [(what's this?)](https://www.cloudbees.com/what-is-buildhive) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-38333755
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
[jclouds-labs-openstack-pull-requests #183](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/183/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-38334147
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
Took a look at RedirectionRetryHandler and it is returning false the first time if the Location header is not present. This is the behavior I wanted. Looking at the code in a href=https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/http/internal/BaseHttpCommandExecutorService.java#L175-L183;BaseHttpCommandExecutorService/a, retry handlers only determine if another retry should be done and modify the HttpCommand to direct to a new endpoint if necessary. Since we don't want to redirect anywhere, the current modification to GlanceErrorHandler is sufficient and correct. I've added the bits to strip off the version in the endpoint if present in order to perform the proper negotiation. Still need to write additional tests, which are: - Test the GlanceRetryHandler - Test the version negotiation if the Keystone catalog returns a versioned endpoint. - Making sure the returned scheme for the URI matches the queried endpoint. - Making sure an error is thrown if the required version does not exist. If I missed a good test, please let me know. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-38334182
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
Using the HttpClient directly or not will depend on whether we are OK about
including that call as part of the GlanceApi or not. Do we want to expose that
method to users? If it makes sense to expose it, we can add a method to the
GlanceApi that returns the versioned endpoints, and call that method instead of
directly using the HttpClient.
Although it seems that it introduces some overhead, Glance does the right thing
returning a 300. By definition that is the status code to return in this kind
of requests.
And regarding the retry handler, that would be possible. The default retry
strategy for redirects is implemented in the
[RedirectionRetryHandler](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/http/handlers/RedirectionRetryHandler.java).
As you see, after checking if the request should be retried, it updates the
HttpCommand with the new request to be performed.
You could subclass it, and, if the request is a version negotiation request,
update the HttpCommand accordingly so the request is re-sent to the right
endpoint. For other requests, you can delegate to the super to keep the default
retry logic. Once you have the custom retry handler, you can add the following
to the `GlanceHttpApiModule` to instruct jclouds to use it:
```java
@Override
protected void bindRetryHandlers() {
bind(HttpRetryHandler.class).annotatedWith(Redirection.class).to(YourCustomRetryHandler.class);
}
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-38030469
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
Many thanks, @markwash! Guess we can't get more official input than that ;-) Looking forward to the changes, @jasdeep-hundal! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-38089395
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
Many thanks for the feedback! If I undertand it properly, although keystone is not capable of doing content negotiation, the approach currently implemented is correct, isn't it? We'll try to connect to the returned one, and negotiate the version if it returns a 300. Is this version negotiation standard so we can move that logic to the common openstack modules? If not, please correct me, but I think it is better to inspect the body returned in the 300 respo se rather than directly replacing the api version? Thx! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-38094567
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+import com.google.common.base.Supplier;
+import com.google.common.base.Throwables;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+
+/**
+ *
+ * @author Jasdeep Hundal
+ */
+@Singleton
+public class ZoneToEndpointNegotiateVersion implements FunctionObject, URI
{
+
+ public static final String VERSION_NEGOTIATION_HEADER =
Is-Version-Negotiation-Request;
+
+ private static class VersionsJsonResponse{
I don't think we keep domain objects such as this one in functions much. Is
this a pattern you've copied from elsewhere in the jclouds code?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r10723873
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ this.zoneToEndpointSupplier = checkNotNull(zoneToEndpointSupplier,
zoneToEndpointSupplier);
+ if(!apiVersionString.startsWith(v))
+ apiVersionString = v + apiVersionString;
+ this.apiVersion = apiVersionString;
+ this.endpointCache = CacheBuilder.newBuilder()
+ .build(
+new CacheLoaderURI, URI() {
+ public URI load(URI baseEndpointUri) {
+ URI versionEndpointUri = null;
+ try {
+ HttpRequest negotiationRequest = HttpRequest.builder()
+.method(GET).endpoint(baseEndpointUri)
+.addHeader(VERSION_NEGOTIATION_HEADER,
true).build();
+ InputStream response =
client.invoke(negotiationRequest).getPayload().openStream();
+ VersionsJsonResponse versions =
json.fromJson(Strings2.toStringAndClose(response),
VersionsJsonResponse.class);
+ for( VersionsJsonResponse.Version version :
versions.versions ) {
Formatting: for (V...s)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r10723963
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ VersionsJsonResponse versions =
json.fromJson(Strings2.toStringAndClose(response),
VersionsJsonResponse.class);
+ for( VersionsJsonResponse.Version version :
versions.versions ) {
+if(apiVersion.equals(version.id)) {
+ String newURIString = version.links.get(0).href;
+ if(newURIString.startsWith(http:)
baseEndpointUri.toString().startsWith(https:))
+ newURIString = https +
newURIString.substring(4);
+ versionEndpointUri = new URI(newURIString);
+ break;
+}
+ }
+ } catch (Exception ex) {
+ throw Throwables.propagate(ex);
+ }
+ if (versionEndpointUri == null)
+ throw new HttpException(Glance endpoint does not
support API version: + apiVersion);
+ return versionEndpointUri;
Why can't we just return the first one we find in the loop?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r10724084
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ break;
+}
+ }
+ } catch (Exception ex) {
+ throw Throwables.propagate(ex);
+ }
+ if (versionEndpointUri == null)
+ throw new HttpException(Glance endpoint does not
support API version: + apiVersion);
+ return versionEndpointUri;
+ }
+ });
+ }
+
+ @Override
+ public URI apply(Object from) {
+ checkArgument(from != null from instanceof String, you must
specify a zone, as a String argument);
We don't need the `!= null` check, null is not `instanceof String`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r10724109
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ }
+ } catch (Exception ex) {
+ throw Throwables.propagate(ex);
+ }
+ if (versionEndpointUri == null)
+ throw new HttpException(Glance endpoint does not
support API version: + apiVersion);
+ return versionEndpointUri;
+ }
+ });
+ }
+
+ @Override
+ public URI apply(Object from) {
+ checkArgument(from != null from instanceof String, you must
specify a zone, as a String argument);
+ MapString, SupplierURI zoneToEndpoint =
zoneToEndpointSupplier.get();
+ checkState(zoneToEndpoint.size() 0, no zone name to endpoint
mappings configured!);
`!zoneToEndpoint.isEmpty()`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r10724130
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
@@ -46,6 +47,11 @@ public void handleError(HttpCommand command, HttpResponse
response) {
message = message != null ? message : String.format(%s - %s,
command.getCurrentRequest().getRequestLine(),
response.getStatusLine());
switch (response.getStatusCode()) {
+ // This is exclusively to not throw exceptions on Glance version
negotiation
+ case 300:
+if
(command.getCurrentRequest().getFirstHeaderOrNull(ZoneToEndpointNegotiateVersion.VERSION_NEGOTIATION_HEADER)
!= null)
Static import `ZoneToEndpointNegotiateVersion.VERSION_NEGOTIATION_HEADER`? But
what causes the 300 response from Glance in the firest place?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r10724608
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+import com.google.common.base.Supplier;
+import com.google.common.base.Throwables;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+
+/**
+ *
+ * @author Jasdeep Hundal
+ */
+@Singleton
+public class ZoneToEndpointNegotiateVersion implements FunctionObject, URI
{
+
+ public static final String VERSION_NEGOTIATION_HEADER =
Is-Version-Negotiation-Request;
+
+ private static class VersionsJsonResponse{
Nope, this was just something that works. Didn't know the JClouds way of doing
things here, open to suggestions.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r10724655
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ public String status;
+ public String id;
+ public ListLink links;
+ }
+ public ListVersion versions;
+ }
+
+ private final SupplierMapString, SupplierURI zoneToEndpointSupplier;
+ private final String apiVersion;
+ private final LoadingCacheURI, URI endpointCache;
+
+ @Inject
+ public ZoneToEndpointNegotiateVersion(@Zone SupplierMapString,
SupplierURI zoneToEndpointSupplier,
+ @ApiVersion String apiVersionString, final HttpClient client, final
Json json) {
+ this.zoneToEndpointSupplier = checkNotNull(zoneToEndpointSupplier,
zoneToEndpointSupplier);
+ if(!apiVersionString.startsWith(v))
Probably was being too cautious here, didn't know if folks would ever specify a
version with just the number.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r10724727
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
@@ -0,0 +1,44 @@
+{
[minor] I think we normally use smaller indents for JSON files? I may be wrong,
though.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r10724813
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
I think we're missing some tests for this new functionality: what should happen if the API version is/is not found, validating the http - https switch etc. I'm not sure, though, whether the explicit use of HttpClient, as opposed to adding the version negotiation call as an explicit API call elsewhere and then invoking that, is what we want? @nacx: thoughts on that? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-37986049
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+import com.google.common.base.Supplier;
+import com.google.common.base.Throwables;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+
+/**
+ *
+ * @author Jasdeep Hundal
+ */
+@Singleton
+public class ZoneToEndpointNegotiateVersion implements FunctionObject, URI
{
+
+ public static final String VERSION_NEGOTIATION_HEADER =
Is-Version-Negotiation-Request;
+
+ private static class VersionsJsonResponse{
@nacx: do you have an example handy? ;-) But whether we need this or not will
largely depend on the answer to the use of HttpClient question, I guess...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r10725021
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ public String status;
+ public String id;
+ public ListLink links;
+ }
+ public ListVersion versions;
+ }
+
+ private final SupplierMapString, SupplierURI zoneToEndpointSupplier;
+ private final String apiVersion;
+ private final LoadingCacheURI, URI endpointCache;
+
+ @Inject
+ public ZoneToEndpointNegotiateVersion(@Zone SupplierMapString,
SupplierURI zoneToEndpointSupplier,
+ @ApiVersion String apiVersionString, final HttpClient client, final
Json json) {
+ this.zoneToEndpointSupplier = checkNotNull(zoneToEndpointSupplier,
zoneToEndpointSupplier);
+ if(!apiVersionString.startsWith(v))
Then I'd say let's be tough, and simply add a `checkArgument` the requires an
initial v and an appropriate doc comment (no need for a test, I think).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r10725056
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ break;
+}
+ }
+ } catch (Exception ex) {
+ throw Throwables.propagate(ex);
+ }
+ if (versionEndpointUri == null)
+ throw new HttpException(Glance endpoint does not
support API version: + apiVersion);
+ return versionEndpointUri;
+ }
+ });
+ }
+
+ @Override
+ public URI apply(Object from) {
+ checkArgument(from != null from instanceof String, you must
specify a zone, as a String argument);
This and the next bit were copied from the ZoneToEndpoint class in jclouds
core. (Just a note in case you want to make the fix there as well).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r10725241
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
@@ -0,0 +1,44 @@
+{
I think it was actually 4 in the keystoneAuthResponse.json, but I'm not
consistent with that either here...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r10725757
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ public String status;
+ public String id;
+ public ListLink links;
+ }
+ public ListVersion versions;
+ }
+
+ private final SupplierMapString, SupplierURI zoneToEndpointSupplier;
+ private final String apiVersion;
+ private final LoadingCacheURI, URI endpointCache;
+
+ @Inject
+ public ZoneToEndpointNegotiateVersion(@Zone SupplierMapString,
SupplierURI zoneToEndpointSupplier,
+ @ApiVersion String apiVersionString, final HttpClient client, final
Json json) {
+ this.zoneToEndpointSupplier = checkNotNull(zoneToEndpointSupplier,
zoneToEndpointSupplier);
+ if(!apiVersionString.startsWith(v))
The tricky bit for me was that since we're using the apiVersion field in an
anonymous class that we construct, the compiler needs to know that the field is
final, and the compiler wasn't able to disambiguate between a nonfinal
parameter in the constructor and the final field in the class that both had the
same name.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r10726147
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
[jclouds » jclouds-labs-openstack #927](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/927/) UNSTABLE Looks like there's a problem with this pull request [(what's this?)](https://www.cloudbees.com/what-is-buildhive) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-37992059
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
[jclouds-labs-openstack-pull-requests #182](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/182/) UNSTABLE Looks like there's a problem with this pull request --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-37992211
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
Thinking about it some more, would it be possible/cleaner to use a retryHandler instead of an errorHandler to deal with the HttpClient behavior on HTTP responses with a 300 status code? Well, the cleanest thing I can think of would be to hook in where the success/failure of the call is determined, and decide that 300 is actually a success response code for this particular call. But not sure how we would go about that. On the note of an explicit use of HttpClient, I don't think the Image/GlanceApi is where this should go Agreed. But what is the OpenStack API that we're actually talking to here? Some kind of admin/config API? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/82#issuecomment-38003773
