andreaturli commented on this pull request.
looks really useful as we discussed on IRC.
Some minor comments, but @geomacy your impl is even cleaner than expected using
the annotation.
Thanks!
> @@ -101,7 +105,25 @@
this.serviceAndRegion = serviceAndRegion;
}
- @Override public HttpRequest filter(HttpRequest request) throws
HttpException {
+ private String getAnnotatedApiVersion(HttpRequest request) {
[minor] Although private could you make return `Optional<String>` instead of
null ?
> +
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+import javax.inject.Qualifier;
+
+
+/**
+ * Designates that a method overrides the {@link ApiVersion} on the class with
a specific value.
+ *
+ * @see ApiVersion
+ */
+@Target({ METHOD })
+@Retention(RUNTIME)
+@Qualifier
+public @interface ApiVersionOverride {
I like this idea, although maybe [ARM
approach](https://github.com/andreaturli/jclouds-labs/blob/master/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/AzureComputeProviderMetadata.java#L100)
-- which is meant to solve the same issue -- is slightly more flexible as you
can configure the version without re-building the project.
Anyway I think it is really unlikely that a new version would not require a
change at level of java implementation, so I think the Annotation is sensible
here.
@nacx any further comments?
> @@ -130,4 +133,24 @@ void deleteSubnetInRegion(
FluentIterable<Subnet> describeSubnetsInRegionWithFilter(
@EndpointParam(parser = RegionToEndpointOrProviderIfNull.class)
@Nullable String region,
@BinderParam(BindFiltersToIndexedFormParams.class) Multimap<String,
String> filter);
+
+ /**
+ * Modifies a subnet attribute. You can only modify one attribute at a time.
+ *
+ * @param region The region for the subnet
+ * @param subnetId The ID of the subnet
+ * @param options The options containing the attribute to modify. You can
only modify one attribute at a time.
+ * @return true if the modification was successful
+ */
+ @ApiVersionOverride("2014-06-15")
neat, I like it!
--
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/1102#pullrequestreview-38925377
