nacx requested changes on this pull request.
Just some minor final comments, but it looks quite good. Thanks,
@trevorflanagan!
> + this.state = state;
+ }
+
+ @Override
+ public boolean apply(String networkDomainId) {
+ checkNotNull(networkDomainId, "networkDomainId");
+ logger.trace("looking for state on network domain %s", networkDomainId);
+ final NetworkDomain networkDomain =
networkApi.getNetworkDomain(networkDomainId);
+
+ if (networkDomain == null && state == State.DELETED) {
+ return true;
+ }
+
+ if (networkDomain.state().isFailed()) {
+ throw new IllegalStateException(
+ MessageFormat.format("Network Domain {0} is in FAILED state",
networkDomain.id()));
This looks like more a check for the caller? What if the expected state passed
to the constructor is a "failed" one? I mean, although it is unlikely we want
individual classes to have meaningful and complete method signatures.
> + this.networkApi = networkApi;
+ this.state = state;
+ }
+
+ @Override
+ public boolean apply(String vlanId) {
+ checkNotNull(vlanId, "vlanId");
+ logger.trace("looking for state on vlan %s", vlanId);
+ final Vlan vlan = networkApi.getVlan(vlanId);
+
+ if (vlan == null && state == State.DELETED) {
+ return true;
+ }
+
+ if (vlan.state().isFailed()) {
+ throw new IllegalStateException(MessageFormat.format("Vlan {0} is in
FAILED state", vlan.id()));
Same comment as above.
> +import org.jclouds.json.Json;
+import org.jclouds.logging.Logger;
+
+import javax.annotation.Resource;
+import java.io.InputStream;
+
+import static org.jclouds.http.HttpUtils.releasePayload;
+
+public class ParseResponse implements Function<HttpResponse, String> {
+
+ @Resource
+ protected Logger logger = Logger.NULL;
+ protected final Json json;
+ protected String propertyName;
+
+ public ParseResponse(Json json, String propertyName) {
Make this protected?
> +import org.jclouds.http.HttpResponse;
+import org.jclouds.http.HttpResponseException;
+import org.jclouds.json.Json;
+import org.jclouds.logging.Logger;
+
+import javax.annotation.Resource;
+import java.io.InputStream;
+
+import static org.jclouds.http.HttpUtils.releasePayload;
+
+public class ParseResponse implements Function<HttpResponse, String> {
+
+ @Resource
+ protected Logger logger = Logger.NULL;
+ protected final Json json;
+ protected String propertyName;
Make it final too.
> + String tryFindInfoPropertyValue(Response response) {
+ if (!response.info().isEmpty()) {
+ Optional<String> optionalPropertyName =
FluentIterable.from(response.info())
+ .firstMatch(new Predicate<Property>() {
+ @Override
+ public boolean apply(Property input) {
+ return input.name().equals(propertyName);
+ }
+ }).transform(new Function<Property, String>() {
+ @Override
+ public String apply(Property input) {
+ return input.value();
+ }
+ });
+ if (!optionalPropertyName.isPresent()) {
+ throw new IllegalStateException();
Add a message here so the HttpResponseException you propagate above has all the
info.
--
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-labs/pull/389#pullrequestreview-45907911
