You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Rita Zhang <no...@github.com> on 2016/04/14 01:28:12 UTC

[jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

@nacx @andreaturli 
This PR is for StorageAccountApi, part of the foundations needed for interacting with Azure Resource Manager (ARM) Rest APIs.

FYI
@jmspring @jtjk @isalento
You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds-labs/pull/259

-- Commit Summary --

  * JCLOUDS-664 Azurecompute-arm StorageAccountApi

-- File Changes --

    M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/AzureComputeApi.java (12)
    A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Availability.java (32)
    A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/CreateStorageServiceParams.java (65)
    A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/StorageService.java (208)
    A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/StorageServiceKeys.java (39)
    A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/StorageServiceUpdateParams.java (185)
    A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/features/StorageAccountApi.java (160)
    A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/functions/StatusCodeParser.java (37)
    A azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/StorageAccountApiLiveTest.java (134)
    A azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/StorageAccountApiMockTest.java (288)
    M azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/internal/BaseAzureComputeApiLiveTest.java (58)
    A azurecompute-arm/src/test/resources/isavailablestorageservice.json (3)
    A azurecompute-arm/src/test/resources/storageAccounts.json (90)
    A azurecompute-arm/src/test/resources/storageCreateResponse.json (9)
    A azurecompute-arm/src/test/resources/storageaccountkeys.json (4)
    A azurecompute-arm/src/test/resources/storageaccountupdate.json (7)
    A azurecompute-arm/src/test/resources/storageservices.json (30)

-- Patch Links --

https://github.com/jclouds/jclouds-labs/pull/259.patch
https://github.com/jclouds/jclouds-labs/pull/259.diff

---
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/259

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

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

---
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/259#event-633031641

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +   public void testUpdate() throws Exception {
> +      server.enqueue(jsonResponse("/storageaccountupdate.json"));
> +
> +      final StorageAccountApi storageAPI = api.getStorageAccountApi(resourceGroup);
> +
> +      StorageServiceUpdateParams.StorageServiceUpdateProperties props =
> +      StorageServiceUpdateParams.StorageServiceUpdateProperties.create(null, null, null, null, null, null, null, null,
> +              null);
> +
> +      final StorageServiceUpdateParams params = storageAPI.update("TESTSTORAGE", props,
> +              ImmutableMap.of("another_property_name", "another_property_value"));
> +
> +      assertTrue(params.tags().containsKey("another_property_name"));
> +
> +      assertSent(server, "PATCH", "/subscriptions/" + subsriptionId + "/resourcegroups/" + resourceGroup +
> +         "/providers/Microsoft.Storage/storageAccounts/TESTSTORAGE?api-version=2015-06-15");

Add body check.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59686405

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +              "provisioningState", "secondaryEndpoints", "secondaryLocation", "statusOfPrimary", "statusOfSecondary"})
> +      public static StorageServiceUpdateProperties create(final AccountType accountType, final Date creationTime,
> +              final Map<String, String> primaryEndpoints, final String primaryLocation, final Status provisioningState,
> +              final Map<String, String> secondaryEndpoints, final String secondaryLocation,
> +              final RegionStatus statusOfPrimary, final RegionStatus statusOfSecondary) {
> +
> +         return new AutoValue_StorageServiceUpdateParams_StorageServiceUpdateProperties(accountType, creationTime,
> +                 primaryEndpoints == null ? null : ImmutableMap.copyOf(primaryEndpoints), primaryLocation, provisioningState,
> +                 secondaryEndpoints == null ? null : ImmutableMap.copyOf(secondaryEndpoints), secondaryLocation, statusOfPrimary, statusOfSecondary);
> +      }
> +   }
> +
> +   /**
> +    * Specifies the tags of the storage account.
> +    */
> +   @Nullable

Remove.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59683741

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Jim Spring <no...@github.com>.
@nacx Using 204 for a fallback is not going to work, from my understanding of the code.  If you step through BaseHttpCommandExecutorService.invoke where the HTTP request is made.  Only in the path where an error code is not 2XX, is the path to check for fallbacks triggered.

Specifically, the BaseHttpCommandExecutorService.shouldContinue method checks for the need to continue or not, if not, an error is generated and that error is then propagated back at the end of the BaseHttpCommandExecutorService.invoke method and the Fallback triggered.

Thoughts?

---
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/259#issuecomment-210853819

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Rita Zhang <no...@github.com>.
> +public class StorageAccountApiMockTest extends BaseAzureComputeApiMockTest {
> +
> +   private String subsriptionId = "SUBSCRIPTIONID";
> +   private String resourceGroup = "resourceGroup";
> +
> +   public void testList() throws Exception {
> +      server.enqueue(jsonResponse("/storageAccounts.json"));
> +
> +      final StorageAccountApi storageAPI = api.getStorageAccountApi(resourceGroup);
> +
> +      List<StorageService> list = storageAPI.list();
> +      assertEquals(list, expected());
> +
> +      assertSent(server, "GET", "/subscriptions/" + subsriptionId +
> +              "/resourcegroups/resourceGroup/providers/Microsoft.Storage/storageAccounts?api-version=2015-06-15");
> +   }

Added.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59813670

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Rita Zhang <no...@github.com>.
> +   }
> +
> +   @Test(dependsOnMethods = "testIsAvailable")
> +   public void testCreate() {
> +
> +      CreateStorageServiceParams storage = api().create(NAME, LOCATION, ImmutableMap.of("property_name",
> +              "property_value"), ImmutableMap.of("accountType", StorageService.AccountType.Standard_ZRS.toString()));
> +      while (storage == null) {
> +         try {
> +            Thread.sleep(25 * 1000);
> +         } catch (InterruptedException e) {
> +            e.printStackTrace();
> +         }
> +         storage = api().create(NAME, LOCATION, ImmutableMap.of("property_name", "property_value"),
> +                 ImmutableMap.of("accountType", StorageService.AccountType.Standard_ZRS.toString()));
> +      }

Using `URIParser` and `ParseJobStatus` since this returns 202 then 200 when done. 
```java
URI uri = api().create(NAME, LOCATION, ImmutableMap.of("property_name",
              "property_value"), ImmutableMap.of("accountType", StorageService.AccountType.Standard_ZRS.toString()));
      if (uri != null){
         assertTrue(uri.toString().contains("api-version"));

         boolean jobDone = Predicates2.retry(new Predicate<URI>() {
            @Override public boolean apply(URI uri) {
               return ParseJobStatus.JobStatus.DONE == api.getJobApi().jobStatus(uri);
            }
         }, 60 * 1 * 1000 /* 1 minute timeout */).apply(uri);
         assertTrue(jobDone, "delete operation did not complete in the configured timeout");
      }
      final StorageService service = api().get(NAME);
```

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59809171

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +    * Specifies the name of the storage account. This name is the DNS prefix name and can be used to access blobs,
> +    * queues, and tables in the storage account.
> +    */
> +   @Nullable
> +   public abstract String name();
> +
> +   /**
> +    * Specifies the location of the storage account.
> +    */
> +   @Nullable
> +   public abstract String location();
> +
> +   /**
> +    * Specifies the tags of the storage account.
> +    */
> +   @Nullable

Remove this annotation

---
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/259/files/41981c7a6b99ff8ab2bd536bd913906424563832#r59853240

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +
> +      assertSent(server, "POST", "/subscriptions/" + subsriptionId + "/resourcegroups/" + resourceGroup +
> +              "/providers/Microsoft.Storage/storageAccounts/TESTSTORAGE/listKeys?api-version=2015-06-15");
> +   }
> +
> +   public void testRegenerateKeys() throws Exception {
> +      server.enqueue(jsonResponse("/storageaccountkeys.json"));
> +
> +      final StorageAccountApi storageAPI = api.getStorageAccountApi(resourceGroup);
> +
> +      assertEquals(storageAPI.regenerateKeys("TESTSTORAGE", "key1"), StorageServiceKeys.create(
> +              "bndO7lydwDkMo4Y0mFvmfLyi2f9aZY7bwfAVWoJWv4mOVK6E9c/exLnFsSm/NMWgifLCfxC/c6QBTbdEvWUA7w==",
> +              "/jMLLT3kKqY4K+cUtJTbh7pCBdvG9EMKJxUvaJJAf6W6aUiZe1A1ulXHcibrqRVA2RJE0oUeXQGXLYJ2l85L7A=="));
> +
> +      assertSent(server, "POST", "/subscriptions/" + subsriptionId + "/resourcegroups/" + resourceGroup +
> +              "/providers/Microsoft.Storage/storageAccounts/TESTSTORAGE/regenerateKey?api-version=2015-06-15");

Add body check.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59686383

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.azurecompute.arm.util.GetEnumValue;
> +
> +@AutoValue
> +public abstract class StorageService {
> +
> +   public enum AccountType {
> +
> +      Standard_LRS,
> +      Standard_ZRS,
> +      Standard_GRS,
> +      Standard_RAGRS,
> +      Premium_LRS,
> +      UNRECOGNIZED;
> +
> +       public static AccountType fromValue(final String text) {
> +           return (AccountType) GetEnumValue.fromValueOrDefault(text, AccountType.UNRECOGNIZED);

Is the cast needed? The type should be already resolved by the second parameter.

---
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/259/files/41981c7a6b99ff8ab2bd536bd913906424563832#r59853135

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +       Succeeded,
> +       UNRECOGNIZED;
> +
> +      public static Status fromString(final String text) {
> +         if (text != null) {
> +            for (Status status : Status.values()) {
> +               if (text.equalsIgnoreCase(status.name())) {
> +                  return status;
> +               }
> +            }
> +         }
> +         return UNRECOGNIZED;
> +      }
> +   }
> +
> +   @Nullable

I'm not sure if this annotation has any effect when configured at class level?

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59683678

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Rita Zhang <no...@github.com>.
@nacx Made updates per your feedback. Ready for review. 😄 

---
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/259#issuecomment-210234505

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +              "provisioningState", "secondaryEndpoints", "secondaryLocation", "statusOfPrimary", "statusOfSecondary"})
> +      public static StorageServiceUpdateProperties create(final AccountType accountType, final Date creationTime,
> +              final Map<String, String> primaryEndpoints, final String primaryLocation, final Status provisioningState,
> +              final Map<String, String> secondaryEndpoints, final String secondaryLocation,
> +              final RegionStatus statusOfPrimary, final RegionStatus statusOfSecondary) {
> +
> +         return new AutoValue_StorageServiceUpdateParams_StorageServiceUpdateProperties(accountType, creationTime,
> +                 primaryEndpoints == null ? null : ImmutableMap.copyOf(primaryEndpoints), primaryLocation, provisioningState,
> +                 secondaryEndpoints == null ? null : ImmutableMap.copyOf(secondaryEndpoints), secondaryLocation, statusOfPrimary, statusOfSecondary);
> +      }
> +   }
> +
> +   /**
> +    * Specifies the tags of the storage account.
> +    */
> +   @Nullable

Still needs to be removed.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59853290

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +   }
> +
> +   @Test(dependsOnMethods = "testIsAvailable")
> +   public void testCreate() {
> +
> +      CreateStorageServiceParams storage = api().create(NAME, LOCATION, ImmutableMap.of("property_name",
> +              "property_value"), ImmutableMap.of("accountType", StorageService.AccountType.Standard_ZRS.toString()));
> +      while (storage == null) {
> +         try {
> +            Thread.sleep(25 * 1000);
> +         } catch (InterruptedException e) {
> +            e.printStackTrace();
> +         }
> +         storage = api().create(NAME, LOCATION, ImmutableMap.of("property_name", "property_value"),
> +                 ImmutableMap.of("accountType", StorageService.AccountType.Standard_ZRS.toString()));
> +      }

Which are the semantics of the create operations? It looks kinda weird to attempt the create operation N times instead of calling a `get` operation when waiting.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59685110

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Rita Zhang <no...@github.com>.
> +
> +       /**
> +        * Specifies whether the account supports locally-redundant storage, geo-redundant storage, zone-redundant
> +        * storage, or read access geo-redundant storage.
> +        */
> +       public abstract AccountType accountType();
> +
> +       /**
> +        * Specifies the time that the storage account was created.
> +        */
> +       public abstract Date creationTime();
> +
> +       /**
> +        * Specifies the endpoints of the storage account.
> +        */
> +       @Nullable

@nacx 
So I did remove this annotation in this class. But in order to support create and update requests without tags, I added `@Nullable` in the [StorageAccountApi](https://github.com/ritazh/jclouds-labs/blob/azurecomputearm-storageaccountapi/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/features/StorageAccountApi.java#L90).

---
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/259/files/41981c7a6b99ff8ab2bd536bd913906424563832#r59952027

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +       * The secondary status of the storage account.
> +       */
> +      @Nullable
> +      public abstract RegionStatus statusOfSecondary();
> +
> +
> +      @SerializedNames({"accountType", "creationTime", "primaryEndpoints",  "primaryLocation",
> +              "provisioningState", "secondaryEndpoints", "secondaryLocation", "statusOfPrimary", "statusOfSecondary"})
> +      public static StorageServiceProperties create(final AccountType accountType, final Date creationTime,
> +              final Map<String, String> primaryEndpoints, final String primaryLocation, final Status provisioningState,
> +              final Map<String, String> secondaryEndpoints, final String secondaryLocation,
> +              final RegionStatus statusOfPrimary, final RegionStatus statusOfSecondary) {
> +
> +         return new AutoValue_StorageService_StorageServiceProperties(accountType, creationTime,
> +                 primaryEndpoints, primaryLocation, provisioningState,
> +                 secondaryEndpoints, secondaryLocation, statusOfPrimary, statusOfSecondary);

Do we want to have null maps here? Can't we follow the same pattern and default to empty ones?

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59683238

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +@Test(groups = "live", testName = "StorageAccountApiLiveTest")
> +public class StorageAccountApiLiveTest extends BaseAzureComputeApiLiveTest {
> +
> +   private static final String NAME = String.format("%3.24s",
> +           RAND + StorageAccountApiLiveTest.class.getSimpleName().toLowerCase());
> +
> +   private void check(final StorageService storage) {
> +      assertNotNull(storage.id());
> +      assertNotNull(storage.name());
> +      assertNotNull(storage.storageServiceProperties());
> +      assertNotNull(storage.storageServiceProperties().accountType());
> +      assertFalse(storage.storageServiceProperties().primaryEndpoints().isEmpty());
> +      assertNotNull(storage.storageServiceProperties().creationTime());
> +   }
> +
> +   @Test()

Should this better depend on the create test, to make sure there is content?

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59684889

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Rita Zhang <no...@github.com>.
> +import org.jclouds.azurecompute.arm.util.GetEnumValue;
> +
> +@AutoValue
> +public abstract class StorageService {
> +
> +   public enum AccountType {
> +
> +      Standard_LRS,
> +      Standard_ZRS,
> +      Standard_GRS,
> +      Standard_RAGRS,
> +      Premium_LRS,
> +      UNRECOGNIZED;
> +
> +       public static AccountType fromValue(final String text) {
> +           return (AccountType) GetEnumValue.fromValueOrDefault(text, AccountType.UNRECOGNIZED);

Yes...compiler is returning: "error: incompatible types: Enum<AccountType> cannot be converted to AccountType" without 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-labs/pull/259/files/41981c7a6b99ff8ab2bd536bd913906424563832#r59951178

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Rita Zhang <no...@github.com>.
@nacx Latest commit includes `@ResponseParser(FalseOn204.class)` for `delete`. Ready for another round of review. Thanks!

---
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/259#issuecomment-211514908

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Rita Zhang <no...@github.com>.
> +   public void testList() {
> +      List<StorageService> storages = api().list();
> +      assertTrue(storages.size() > 0);
> +      for (StorageService storage : storages) {
> +         check(storage);
> +      }
> +   }
> +
> +   @Test()
> +   public void testListAll() {
> +      List<StorageService> storages = api().list();
> +      assertTrue(storages.size() > 0);
> +      for (StorageService storage : storages) {
> +         check(storage);
> +      }
> +   }

Removed.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59813594

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.rest.annotations.PayloadParam;
> +import org.jclouds.rest.annotations.MapBinder;
> +import org.jclouds.rest.binders.BindToJsonPayload;
> +
> +import java.util.List;
> +import java.util.Map;
> +
> +/**
> + * The Azure Resource Management API includes operations for managing the storage accounts in your subscription.
> + *
> + * @see <a href="https://msdn.microsoft.com/en-us/library/mt163683.aspx">docs</a>
> + */
> +@Path("/")
> +@RequestFilters(OAuthFilter.class)
> +@QueryParams(keys = "api-version", values = "2015-06-15")
> +@Produces(MediaType.APPLICATION_JSON)

This will set the Content-Type header, which is not used in GET methods. Move this annotation to just the methods that generate a body.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59683902

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +   @MapBinder(BindToJsonPayload.class)
> +   @Path("/resourcegroups/{resourceGroup}/providers/Microsoft.Storage/storageAccounts/{storageAccountName}")
> +   StorageServiceUpdateParams update(
> +           @PathParam("storageAccountName") String storageAccountName,
> +           @PayloadParam("properties") StorageServiceUpdateParams.StorageServiceUpdateProperties properties,
> +           @PayloadParam("tags") Map<String, String> tags);
> +
> +   /**
> +    * https://msdn.microsoft.com/en-us/library/mt163652.aspx
> +    * DELETE
> +    */
> +   @Named("storageaccount:delete")
> +   @DELETE
> +   @ResponseParser(StatusCodeParser.class)
> +   @Path("/resourcegroups/{resourceGroup}/providers/Microsoft.Storage/storageAccounts/{storageAccountName}")
> +   String delete(@PathParam("storageAccountName") String storageAccountName);

Also consider removing the StatusCodeParser class if we go for this approach and it is then not used.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59853361

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +      StorageAccountApi storageApi = api.getStorageAccountApi(getResourceGroupName());
> +      StorageService ss = storageApi.get(storageServiceName);
> +      if (ss != null) {
> +         return ss;
> +      }
> +      URI uri = storageApi.create(storageServiceName, LOCATION, ImmutableMap.of("property_name",
> +              "property_value"), ImmutableMap.of("accountType", StorageService.AccountType.Standard_ZRS.toString()));
> +      if (uri != null){
> +         assertTrue(uri.toString().contains("api-version"));
> +
> +         boolean jobDone = Predicates2.retry(new Predicate<URI>() {
> +            @Override public boolean apply(URI uri) {
> +               return ParseJobStatus.JobStatus.DONE == api.getJobApi().jobStatus(uri);
> +            }
> +         }, 60 * 1 * 1000 /* 1 minute timeout */).apply(uri);
> +         assertTrue(jobDone, "delete operation did not complete in the configured timeout");

"create operation"

---
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/259/files/41981c7a6b99ff8ab2bd536bd913906424563832#r59853585

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +public class StorageAccountApiMockTest extends BaseAzureComputeApiMockTest {
> +
> +   private String subsriptionId = "SUBSCRIPTIONID";
> +   private String resourceGroup = "resourceGroup";
> +
> +   public void testList() throws Exception {
> +      server.enqueue(jsonResponse("/storageAccounts.json"));
> +
> +      final StorageAccountApi storageAPI = api.getStorageAccountApi(resourceGroup);
> +
> +      List<StorageService> list = storageAPI.list();
> +      assertEquals(list, expected());
> +
> +      assertSent(server, "GET", "/subscriptions/" + subsriptionId +
> +              "/resourcegroups/resourceGroup/providers/Microsoft.Storage/storageAccounts?api-version=2015-06-15");
> +   }

Add a test that exercises the 404 fallback for the list method.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59685490

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Jim Spring <no...@github.com>.
Also @nacx - I think there is a benefit of being able to distinguish between a 200 and 204, mostly in the case to allow the caller to maybe catch the case where the resource specified should be there, but maybe the name was wrong.

---
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/259#issuecomment-210912831

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +
> +      assertSent(server, "PUT", "/subscriptions/" + subsriptionId +
> +              "/resourcegroups/resourceGroup/providers/Microsoft.Storage/" +
> +              "storageAccounts/name-of-storage-account?api-version=2015-06-15");
> +   }
> +
> +   public void testIsAvailable() throws Exception {
> +      server.enqueue(jsonResponse("/isavailablestorageservice.json"));
> +
> +      final StorageAccountApi storageAPI = api.getStorageAccountApi(resourceGroup);
> +
> +      assertEquals(storageAPI.isAvailable("TESTSTORAGE"),
> +              Availability.create("true"));
> +
> +      assertSent(server, "POST", "/subscriptions/" + subsriptionId +
> +              "/providers/Microsoft.Storage/checkNameAvailability?api-version=2015-06-15");

Also verify the body here.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59686335

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +            for (AccountType type : AccountType.values()) {
> +               if (text.equalsIgnoreCase(type.name())) {
> +                  return type;
> +               }
> +            }
> +         }
> +         return UNRECOGNIZED;
> +      }
> +   }
> +
> +   public enum RegionStatus {
> +
> +       Available,
> +       Unavailable,
> +       available,
> +       unavailable,

Are the lower case versions really needed?

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59683089

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +
> +       /**
> +        * Specifies whether the account supports locally-redundant storage, geo-redundant storage, zone-redundant
> +        * storage, or read access geo-redundant storage.
> +        */
> +       public abstract AccountType accountType();
> +
> +       /**
> +        * Specifies the time that the storage account was created.
> +        */
> +       public abstract Date creationTime();
> +
> +       /**
> +        * Specifies the endpoints of the storage account.
> +        */
> +       @Nullable

Remove this annotation

---
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/259/files/41981c7a6b99ff8ab2bd536bd913906424563832#r59853213

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +    * Specifies the type of the storage account.
> +    */
> +   @Nullable
> +   public abstract String type();
> +
> +   /**
> +    * Specifies the properties of the storage account.
> +    */
> +   public abstract StorageServiceProperties storageServiceProperties();
> +
> +
> +   @SerializedNames({"id", "name", "location", "tags", "type", "properties"})
> +   public static StorageService create(final String id,  final String name,  final String location,
> +                                       @Nullable final Map<String, String> tags,  final String type,
> +                                       final StorageServiceProperties storageServiceProperties) {
> +      return new AutoValue_StorageService(id,  name,  location,  tags == null ? ImmutableMap.<String, String>builder().build() : ImmutableMap.copyOf(tags), type, storageServiceProperties);

Remove the `@Nullable` annotation from the `tags` member variable.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59683350

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
Pushed to master as [b82378c2](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/b82378c2). Thanks!

---
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/259#issuecomment-211950177

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +      Standard_GRS,
> +      Standard_RAGRS,
> +      Premium_LRS,
> +      UNRECOGNIZED;
> +
> +      public static AccountType fromString(final String text) {
> +         if (text != null) {
> +            for (AccountType type : AccountType.values()) {
> +               if (text.equalsIgnoreCase(type.name())) {
> +                  return type;
> +               }
> +            }
> +         }
> +         return UNRECOGNIZED;
> +      }
> +   }

This method is repeated in every enum. Consider creating a helper class with method to encapsulate this logic. Something like:
```java
@SuppressWarnings("unchecked")
public static <T extends Enum<T>> Enum<T> fromValueOrDefault(String text, Enum<T> defaultValue) {
   if (text != null) {
      EnumSet<T> elements = EnumSet.allOf(defaultValue.getClass());
      for (Enum<T> element : elements) {
         if (text.equalsIgnoreCase(element.name())) {
            return element;
         }
      }
   }
   return defaultValue;
}
```

And then just call it from all enums:
```java
public static AccountType fromValue(final String text) {
   return fromValueOrDefault(text, AccountType.UNRECOGNIZED);
}
```

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59682662

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +   @MapBinder(BindToJsonPayload.class)
> +   @Path("/resourcegroups/{resourceGroup}/providers/Microsoft.Storage/storageAccounts/{storageAccountName}")
> +   StorageServiceUpdateParams update(
> +           @PathParam("storageAccountName") String storageAccountName,
> +           @PayloadParam("properties") StorageServiceUpdateParams.StorageServiceUpdateProperties properties,
> +           @PayloadParam("tags") Map<String, String> tags);
> +
> +   /**
> +    * https://msdn.microsoft.com/en-us/library/mt163652.aspx
> +    * DELETE
> +    */
> +   @Named("storageaccount:delete")
> +   @DELETE
> +   @ResponseParser(StatusCodeParser.class)
> +   @Path("/resourcegroups/{resourceGroup}/providers/Microsoft.Storage/storageAccounts/{storageAccountName}")
> +   String delete(@PathParam("storageAccountName") String storageAccountName);

Looking at the docs, it does not seem to return a URL to track an ongoing delete operation; just 200 or 204. WDYT about just returning `void` and remove the response parser? Any other response status code would then throw an exception.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59684579

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +       Changing,
> +       ResolvingDns,
> +       Succeeded,
> +       UNRECOGNIZED;
> +
> +      public static Status fromString(final String text) {
> +         if (text != null) {
> +            for (Status status : Status.values()) {
> +               if (text.equalsIgnoreCase(status.name())) {
> +                  return status;
> +               }
> +            }
> +         }
> +         return UNRECOGNIZED;
> +      }
> +   }

These enums are the same than the previous ones. Extract them to top-level type classes.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59683593

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +       /**
> +       * A primaryLocation for the storage account.
> +       */
> +      @Nullable
> +      public abstract String primaryLocation();
> +
> +      /**
> +       * provisioningState for the storage group
> +       */
> +      @Nullable
> +      public abstract Status provisioningState();
> +
> +       /**
> +        * Specifies the secondary endpoints of the storage account.
> +        */
> +       @Nullable

Remove this annotation

---
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/259/files/41981c7a6b99ff8ab2bd536bd913906424563832#r59853219

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +    * Represents the name of an cloud service property. Each property must have both a defined name
> +    * and value. You can have a maximum of 50 extended property name/value pairs.
> +    *
> +    * <p/>
> +    * The maximum length of the Name element is 64 characters, only alphanumeric characters and underscores are valid in
> +    * the Name, and the name must start with a letter. Each extended property value has a maximum length of 255
> +    * characters.
> +    */
> +   @Nullable
> +   public abstract Map<String, String> properties();
> +
> +   @SerializedNames({"location", "tags", "properties"})
> +   public static CreateStorageServiceParams create(
> +           final String location, @Nullable final Map<String, String> tags, @Nullable final Map<String, String> properties) {
> +
> +      return new AutoValue_CreateStorageServiceParams(location, tags == null ? ImmutableMap.<String, String>builder().build() : ImmutableMap.copyOf(tags), properties == null ? ImmutableMap.<String, String>builder().build() : ImmutableMap.copyOf(properties));

Since you're enforcing non-null maps here, remove the `@Nullable` annotations from the variables.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59680062

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +      assertSent(server, "GET", "/subscriptions/" + subsriptionId +
> +              "/resourcegroups/resourceGroup/providers/Microsoft.Storage/storageAccounts?api-version=2015-06-15");
> +   }
> +
> +   public void testCreate() throws Exception {
> +      server.enqueue(jsonResponse("/storageCreateResponse.json"));
> +      final StorageAccountApi storageAPI = api.getStorageAccountApi(resourceGroup);
> +
> +      assertEquals(storageAPI.create("name-of-storage-account", "westus",
> +              ImmutableMap.of("property_name", "property_value"),
> +              ImmutableMap.of("accountType", StorageService.AccountType.Premium_LRS.toString())),
> +              getCreateResponse());
> +
> +      assertSent(server, "PUT", "/subscriptions/" + subsriptionId +
> +              "/resourcegroups/resourceGroup/providers/Microsoft.Storage/" +
> +              "storageAccounts/name-of-storage-account?api-version=2015-06-15");

For methods that generate a body we also need to verify that the generated body is the expected one.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59685603

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
Thanks @ritazh!

---
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/259#issuecomment-209844947

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Rita Zhang <no...@github.com>.
> +   @MapBinder(BindToJsonPayload.class)
> +   @Path("/resourcegroups/{resourceGroup}/providers/Microsoft.Storage/storageAccounts/{storageAccountName}")
> +   StorageServiceUpdateParams update(
> +           @PathParam("storageAccountName") String storageAccountName,
> +           @PayloadParam("properties") StorageServiceUpdateParams.StorageServiceUpdateProperties properties,
> +           @PayloadParam("tags") Map<String, String> tags);
> +
> +   /**
> +    * https://msdn.microsoft.com/en-us/library/mt163652.aspx
> +    * DELETE
> +    */
> +   @Named("storageaccount:delete")
> +   @DELETE
> +   @ResponseParser(StatusCodeParser.class)
> +   @Path("/resourcegroups/{resourceGroup}/providers/Microsoft.Storage/storageAccounts/{storageAccountName}")
> +   String delete(@PathParam("storageAccountName") String storageAccountName);

@nacx After adding `@Fallback(Fallbacks.FalseOnNotFoundOr204.class)` for delete, in `StorageAccountApiMockTest`, setting response code to 204 does not seem to work. Same test for `FalseOnNotFoundOr204` works fine. Similarly `FalseOnNotFoundOr422` also never returns false. What am I missing here?

```java
server.enqueue(new MockResponse().setResponseCode(204));

      final StorageAccountApi storageAPI = api.getStorageAccountApi(resourceGroup);

      boolean status = storageAPI.delete("TESTSTORAGE");
      assertFalse(status);
```

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59953738

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +   public void testList() {
> +      List<StorageService> storages = api().list();
> +      assertTrue(storages.size() > 0);
> +      for (StorageService storage : storages) {
> +         check(storage);
> +      }
> +   }
> +
> +   @Test()
> +   public void testListAll() {
> +      List<StorageService> storages = api().list();
> +      assertTrue(storages.size() > 0);
> +      for (StorageService storage : storages) {
> +         check(storage);
> +      }
> +   }

This test is repeated.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59684747

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
Some final comments. Thanks @ritazh!

---
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/259#issuecomment-210400933

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Rita Zhang <no...@github.com>.
> +   @MapBinder(BindToJsonPayload.class)
> +   @Path("/resourcegroups/{resourceGroup}/providers/Microsoft.Storage/storageAccounts/{storageAccountName}")
> +   StorageServiceUpdateParams update(
> +           @PathParam("storageAccountName") String storageAccountName,
> +           @PayloadParam("properties") StorageServiceUpdateParams.StorageServiceUpdateProperties properties,
> +           @PayloadParam("tags") Map<String, String> tags);
> +
> +   /**
> +    * https://msdn.microsoft.com/en-us/library/mt163652.aspx
> +    * DELETE
> +    */
> +   @Named("storageaccount:delete")
> +   @DELETE
> +   @ResponseParser(StatusCodeParser.class)
> +   @Path("/resourcegroups/{resourceGroup}/providers/Microsoft.Storage/storageAccounts/{storageAccountName}")
> +   String delete(@PathParam("storageAccountName") String storageAccountName);

@nacx If we just return void, we are not preserving the response from the server. What if users want to check the status of a DELETE operation? Returning 204 would let them know the storage account they are trying to delete no longer exist? Just a thought...

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59776863

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> Thoughts?

Oh, you're right, and it's my fault for not having properly advised and later detected it in the review. Fallbacks are for "failed" requests, and Response Parsers are for successful ones. In this case, to go for the boolean approach we should simply leave the method wihtout any fallback, as jclouds will [return true for 2xx responses](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/rest/internal/TransformerForRequest.java#L116-L117). By removing the fallback we'd let the method to fail otherwise. Another option would be to create a ["FalseOn204" response parser function](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/rest/annotations/ResponseParser.java) and annotate the method accordingly, so it returns `false` on a 204 response, `true` on 200, and an exception otherwise.

WDYT about implementing it as a response parser? (And apologies for my previous bad advice).

---
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/259#issuecomment-211278581

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +   @MapBinder(BindToJsonPayload.class)
> +   @Path("/resourcegroups/{resourceGroup}/providers/Microsoft.Storage/storageAccounts/{storageAccountName}")
> +   StorageServiceUpdateParams update(
> +           @PathParam("storageAccountName") String storageAccountName,
> +           @PayloadParam("properties") StorageServiceUpdateParams.StorageServiceUpdateProperties properties,
> +           @PayloadParam("tags") Map<String, String> tags);
> +
> +   /**
> +    * https://msdn.microsoft.com/en-us/library/mt163652.aspx
> +    * DELETE
> +    */
> +   @Named("storageaccount:delete")
> +   @DELETE
> +   @ResponseParser(StatusCodeParser.class)
> +   @Path("/resourcegroups/{resourceGroup}/providers/Microsoft.Storage/storageAccounts/{storageAccountName}")
> +   String delete(@PathParam("storageAccountName") String storageAccountName);

The pattern we use in most APIs is to let the delete method return a boolean, and then use a fallback like "false on 404". You could change the method in that way to return false on 204.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59852595

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
I've also pushed [jclouds#e5f65a47](http://git-wip-us.apache.org/repos/asf/jclouds/commit/e5f65a47) to remove the fallback for the 204, as it won't never apply. Apologies for my confusion!

---
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/259#issuecomment-211965672

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +      assertTrue(api().isAvailable(NAME).nameAvailable().equals("true"));
> +   }
> +
> +   @Test(dependsOnMethods = "testIsAvailable")
> +   public void testCreate() {
> +      URI uri = api().create(NAME, LOCATION, ImmutableMap.of("property_name",
> +              "property_value"), ImmutableMap.of("accountType", StorageService.AccountType.Standard_ZRS.toString()));
> +      if (uri != null){
> +         assertTrue(uri.toString().contains("api-version"));
> +
> +         boolean jobDone = Predicates2.retry(new Predicate<URI>() {
> +            @Override public boolean apply(URI uri) {
> +               return ParseJobStatus.JobStatus.DONE == api.getJobApi().jobStatus(uri);
> +            }
> +         }, 60 * 1 * 1000 /* 1 minute timeout */).apply(uri);
> +         assertTrue(jobDone, "delete operation did not complete in the configured timeout");

"create operation"

---
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/259/files/41981c7a6b99ff8ab2bd536bd913906424563832#r59853425

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +       * The secondary status of the storage account.
> +       */
> +      @Nullable
> +      public abstract RegionStatus statusOfSecondary();
> +
> +
> +      @SerializedNames({"accountType", "creationTime", "primaryEndpoints", "primaryLocation",
> +              "provisioningState", "secondaryEndpoints", "secondaryLocation", "statusOfPrimary", "statusOfSecondary"})
> +      public static StorageServiceUpdateProperties create(final AccountType accountType, final Date creationTime,
> +              final Map<String, String> primaryEndpoints, final String primaryLocation, final Status provisioningState,
> +              final Map<String, String> secondaryEndpoints, final String secondaryLocation,
> +              final RegionStatus statusOfPrimary, final RegionStatus statusOfSecondary) {
> +
> +         return new AutoValue_StorageServiceUpdateParams_StorageServiceUpdateProperties(accountType, creationTime,
> +                 primaryEndpoints == null ? null : ImmutableMap.copyOf(primaryEndpoints), primaryLocation, provisioningState,
> +                 secondaryEndpoints == null ? null : ImmutableMap.copyOf(secondaryEndpoints), secondaryLocation, statusOfPrimary, statusOfSecondary);

Same about nullable Maps. Remove the annotation if they're initialized to empty.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59683722

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm StorageAccountApi (#259)

Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.javax.annotation.Nullable;
> +import org.jclouds.json.SerializedNames;
> +
> +@AutoValue
> +public abstract class StorageService {
> +
> +   public enum AccountType {
> +
> +      Standard_LRS,
> +      Standard_ZRS,
> +      Standard_GRS,
> +      Standard_RAGRS,
> +      Premium_LRS,
> +      UNRECOGNIZED;
> +
> +      public static AccountType fromString(final String text) {

Better rename to `fromValue` so we can benefit from it during deserialization. jclouds will call the `fromValue` when deserializing values if the default `enum.valueOf` fails (see [EnumTypeAdapterThatReturnsFromValue](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/json/internal/EnumTypeAdapterThatReturnsFromValue.java), the default enum type adapter used in jclouds).

I've just noticed this, but it would be great if you could rename any other similar methods in the enums introduced in the other ARM PRs, if any.

---
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/259/files/545a9a572a375fa04f8b638e5693bb5bf29471f3#r59680912