You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Andrea Turli <no...@github.com> on 2016/08/11 09:03:16 UTC

[jclouds/jclouds-labs] add support for whitelisting locations (#308)

- change location scope to ZONE vs REGION
- edit the README
- fix Region.byName
- add more Regions in Region class
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * add support for whitelisting locations

-- File Changes --

    M azurecompute-arm/README.md (23)
    M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/AzureComputeServiceAdapter.java (115)
    M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/functions/LocationToLocation.java (16)
    M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/config/AzureComputeHttpApiModule.java (15)
    M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Region.java (16)
    M azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/compute/AzureComputeServiceLiveTest.java (27)
    M azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/internal/AzureLiveTestUtils.java (3)
    A azurecompute-arm/src/test/resources/logback-test.xml (34)
    D azurecompute-arm/src/test/resources/logback.xml (82)

-- Patch Links --

https://github.com/jclouds/jclouds-labs/pull/308.patch
https://github.com/jclouds/jclouds-labs/pull/308.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/308

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
@andreaturli pushed 2 commits.

12f49f2  add parser module
8f403aa  separate the DeploymentToVMDeployment to a function


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/308/files/aaae68457526b198a2b30d7f648b4ef0b46ec359..8f403aa5cc1c711496a670c3fbf3e03eadca969e

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Ignasi Barrera <no...@github.com>.
I don't think so. The last comments have not been addressed. We agreed that the change form region to zone was not good, so all that stuff needs to be reverted.

-- 
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/308#issuecomment-246971336

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
> @@ -28,6 +30,7 @@ public static Properties defaultProperties(Properties properties) {
>         properties.put("oauth.credential", "password");
>         properties.put("oauth.endpoint", "https://login.microsoftonline.com/oauth2/token");
>         properties.put(CREDENTIAL_TYPE, CLIENT_CREDENTIALS_SECRET.toString());
> +       properties.put(PROPERTY_ZONES, "northeurope,eastus");

```
   CENTRAL_US("Central US", "US-IA"),
   EAST_US("East US", "US-VA"),
   EAST_US_2("East US 2", "US-VA"),
   US_GOV_IOWA("US Gov Iowa", "US-IA"),
   US_GOV_VIRGINIA("US Gov Virginia", "US-VA"),
   NORTH_CENTRAL_US("North Central US", "US-IL"),
   SOUTH_CENTRAL_US("South Central US", "US-TX"),
   WEST_US("West US", "US-CA"),
   NORTH_EUROPE("North Europe", "IE"),
   WEST_EUROPE("West Europe", "NL"),
   EAST_ASIA("East Asia", "HK"),
   SOUTH_EAST_ASIA("Southeast Asia", "SG"),
   JAPAN_EAST("Japan East", "JP-11"),
   JAPAN_WEST("Japan West", "JP-27"),
   BRAZIL_SOUTH("Brazil South", "BR"),
   AUSTRALIA_EAST("Australia East", "AU-NSW"),
   AUSTRALIA_SOUTH_EAST("Australia Southeast", "AU-VIC"),
   INDIA_CENTRAL("Central India", "IN-GA"),
   INDIA_SOUTH("South India", "IN-TN"),
   INDIA_WEST("West India", "IN-MH"),
   CHINA_EAST("China East", "CN-SH"),
   CHINA_NORTH("China North", "CN-BJ"),
   CANADA_CENTRAL("Canada Central", "CA-ON"),
   CANADA_EAST("Canada East", "CA-QC");
```

-- 
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/308/files/961db5c145342971726063e25776e0011ab8ed36#r74551789

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
> @@ -28,6 +30,7 @@ public static Properties defaultProperties(Properties properties) {
>         properties.put("oauth.credential", "password");
>         properties.put("oauth.endpoint", "https://login.microsoftonline.com/oauth2/token");
>         properties.put(CREDENTIAL_TYPE, CLIENT_CREDENTIALS_SECRET.toString());
> +       properties.put(PROPERTY_ZONES, "northeurope,eastus");

those are the region names according to http://azure.microsoft.com/en-us/regions

-- 
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/308/files/961db5c145342971726063e25776e0011ab8ed36#r74430909

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
andreaturli commented on this pull request.



>     @Inject
    AzureComputeServiceAdapter(final AzureComputeApi api, final AzureComputeConstants azureComputeConstants,
-                              CleanupResources cleanupResources) {
+                              CleanupResources cleanupResources, ProviderMetadata providerMetadata) {

yes, but it doesn't work as well

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

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
> @@ -46,9 +44,9 @@ protected void bindErrorHandlers() {
>     @Override
>     protected void installLocations() {
>        super.installLocations();
> -      bind(ImplicitLocationSupplier.class).
> -              to(OnlyLocationOrFirstRegionOptionallyMatchingRegionId.class).

@nacx I'm having issues in restoring `OnlyLocationOrFirstRegionOptionallyMatchingRegionId` as it fall-back to `providerUri`which is always the same for each region. Wrong supplier?

-- 
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/308/files/961db5c145342971726063e25776e0011ab8ed36#r78728598

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Ignasi Barrera <no...@github.com>.
>  
>  import static com.google.common.base.Preconditions.checkNotNull;
> -import org.jclouds.logging.slf4j.config.SLF4JLoggingModule;
> -import org.jclouds.logging.config.LoggingModule;
> -
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.RESOURCE_GROUP_NAME;
> +import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_SCRIPT_COMPLETE;
> +import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_NODE_RUNNING;
> +import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_PORT_OPEN;
> +import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_NODE_TERMINATED;
> +import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_NODE_SUSPENDED;
>  
>  /**
>   * Live tests for the {@link org.jclouds.compute.ComputeService} integration.

Remove this class from the change set

-- 
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/308/files/961db5c145342971726063e25776e0011ab8ed36#r74422773

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Ignasi Barrera <no...@github.com>.
>     @Inject
>     AzureComputeServiceAdapter(final AzureComputeApi api, final AzureComputeConstants azureComputeConstants,
> -                              CleanupResources cleanupResources) {
> +                              CleanupResources cleanupResources, ProviderMetadata providerMetadata) {

Can you try removing it from the constructor and put the annotation in the class variable together with an `@Inject(optional = true)`?

-- 
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/308/files/961db5c145342971726063e25776e0011ab8ed36#r74608252

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Ignasi Barrera <no...@github.com>.
>     @Inject
>     AzureComputeServiceAdapter(final AzureComputeApi api, final AzureComputeConstants azureComputeConstants,
> -                              CleanupResources cleanupResources) {
> +                              CleanupResources cleanupResources, ProviderMetadata providerMetadata) {

Better inject directly the `@Nullable @Named(LocationConstants.PROPERTY_ZONES) String enabledZones` and split the property and assign the list in the constructor.

-- 
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/308/files/961db5c145342971726063e25776e0011ab8ed36#r74422374

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
andreaturli commented on this pull request.



> @@ -28,6 +30,7 @@ public static Properties defaultProperties(Properties properties) {
        properties.put("oauth.credential", "password");
        properties.put("oauth.endpoint", "https://login.microsoftonline.com/oauth2/token");
        properties.put(CREDENTIAL_TYPE, CLIENT_CREDENTIALS_SECRET.toString());
+       properties.put(PROPERTY_REGIONS, "northeurope,eastus");

Not sure I understand this comment?

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

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.



> -            vmLocations.addAll(m.locations());
-            break;
-         }
-      }
-
-      Iterable<Location> result = Iterables.filter(locations, new Predicate<Location>() {
-         @Override
-         public boolean apply(Location input) {
-            return vmLocations.contains(input.displayName());
-         }
-      });
-
-      return result;
+   private Iterable<String> findWhiteListOfRegions() {
+      if (providerMetadata.getDefaultProperties().get(LocationConstants.PROPERTY_REGIONS) == null)  return null;
+      return Splitter.on(",").trimResults().split((CharSequence) providerMetadata.getDefaultProperties().get(LocationConstants.PROPERTY_REGIONS));

Is this cast to CharSequence 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/308#pullrequestreview-564604

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Ignasi Barrera <no...@github.com>.
> @@ -47,20 +47,14 @@
>     @Override
>     public org.jclouds.domain.Location apply(final Location location) {
>        final LocationBuilder builder = new LocationBuilder();
> -      String id = location.id();
> -      int index = id.lastIndexOf('/');
> -      if (index > 0 && (index + 1) < id.length())
> -         id = id.substring(index + 1);
> -      builder.id(id);
> +      builder.id(location.name());
>        builder.description(location.displayName());
>        builder.parent(getOnlyElement(justProvider.get()));

The parent location of the ZONE should be the REGION instead of the provider?

-- 
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/308/files/961db5c145342971726063e25776e0011ab8ed36#r74422538

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
ok thanks, I guess this one confused me https://github.com/jclouds/jclouds-labs/pull/308#issuecomment-239163128 :)

-- 
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/308#issuecomment-246973355

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
Closed #308.

-- 
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/308#event-795734646

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Ignasi Barrera <no...@github.com>.
>     @Inject
>     AzureComputeServiceAdapter(final AzureComputeApi api, final AzureComputeConstants azureComputeConstants,
> -                              CleanupResources cleanupResources) {
> +                              CleanupResources cleanupResources, ProviderMetadata providerMetadata) {

Does it complain too if you annotate it `@Nullable` as suggested?

-- 
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/308/files/961db5c145342971726063e25776e0011ab8ed36#r74552021

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
@andreaturli pushed 1 commit.

aaae684  revert ZONES to REGIONS


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/308/files/961db5c145342971726063e25776e0011ab8ed36..aaae68457526b198a2b30d7f648b4ef0b46ec359

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.



> @@ -300,28 +288,43 @@ public VMImage getImage(final String id) {
 
    @Override
    public Iterable<Location> listLocations() {
+      final Iterable<String> whiteListZoneName = findWhiteListOfRegions();

Also, better process this in the constructor and assign as a final variable of this class.

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

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Ignasi Barrera <no...@github.com>.
> @@ -28,6 +30,7 @@ public static Properties defaultProperties(Properties properties) {
>         properties.put("oauth.credential", "password");
>         properties.put("oauth.endpoint", "https://login.microsoftonline.com/oauth2/token");
>         properties.put(CREDENTIAL_TYPE, CLIENT_CREDENTIALS_SECRET.toString());
> +       properties.put(PROPERTY_ZONES, "northeurope,eastus");

Yes. If they are regions, then keep the region naming unchanged.

-- 
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/308/files/961db5c145342971726063e25776e0011ab8ed36#r74441733

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.



> @@ -46,9 +44,9 @@ protected void bindErrorHandlers() {
    @Override
    protected void installLocations() {
       super.installLocations();
-      bind(ImplicitLocationSupplier.class).
-              to(OnlyLocationOrFirstRegionOptionallyMatchingRegionId.class).

I've had a deeper look at this and I agree with you.

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

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
@andreaturli pushed 1 commit.

961db5c  address initial comments from @nacx


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/308/files/bea9005a1dbad01499584a1932c4445b64a60659..961db5c145342971726063e25776e0011ab8ed36

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
> @@ -46,9 +44,9 @@ protected void bindErrorHandlers() {
>     @Override
>     protected void installLocations() {
>        super.installLocations();
> -      bind(ImplicitLocationSupplier.class).
> -              to(OnlyLocationOrFirstRegionOptionallyMatchingRegionId.class).

I think `FirstRegion` as `ImplicitLocationSupplier` is a better choice 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/308/files/961db5c145342971726063e25776e0011ab8ed36#r78753512

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
>     @Inject
>     AzureComputeServiceAdapter(final AzureComputeApi api, final AzureComputeConstants azureComputeConstants,
> -                              CleanupResources cleanupResources) {
> +                              CleanupResources cleanupResources, ProviderMetadata providerMetadata) {

I think in this way the PROPERTY_ZONES becomes mandatory (otherwise Guice is not happy)
Is it what we want?

-- 
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/308/files/961db5c145342971726063e25776e0011ab8ed36#r74551457

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
>     @Inject
>     AzureComputeServiceAdapter(final AzureComputeApi api, final AzureComputeConstants azureComputeConstants,
> -                              CleanupResources cleanupResources) {
> +                              CleanupResources cleanupResources, ProviderMetadata providerMetadata) {

According to https://github.com/google/guice/wiki/UseNullable I've also tried javax.annotation.Nullable which requires
```
    <dependency>
      <groupId>com.google.code.findbugs</groupId>
      <artifactId>jsr305</artifactId>
    </dependency>
```
but no luck as well

-- 
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/308/files/961db5c145342971726063e25776e0011ab8ed36#r74560449

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

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

-- 
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/308#issuecomment-239163128

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
merged at (master)[http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/89cb6f0f]

-- 
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/308#issuecomment-248317509

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Ignasi Barrera <no...@github.com>.
nacx requested changes on this pull request.

Thanks @andreaturli! I've added the comments for the things that I think we have to change.

>     @Inject
    AzureComputeServiceAdapter(final AzureComputeApi api, final AzureComputeConstants azureComputeConstants,
-                              CleanupResources cleanupResources) {
+                              CleanupResources cleanupResources, ProviderMetadata providerMetadata) {

Have you tried this?

> @@ -300,28 +288,43 @@ public VMImage getImage(final String id) {
 
    @Override
    public Iterable<Location> listLocations() {
+      final Iterable<String> whiteListZoneName = findWhiteListOfRegions();

Rename to `whiteListedRegionNames`?

> -            vmLocations.addAll(m.locations());
-            break;
-         }
-      }
-
-      Iterable<Location> result = Iterables.filter(locations, new Predicate<Location>() {
-         @Override
-         public boolean apply(Location input) {
-            return vmLocations.contains(input.displayName());
-         }
-      });
-
-      return result;
+   private Iterable<String> findWhiteListOfRegions() {
+      if (providerMetadata.getDefaultProperties().get(LocationConstants.PROPERTY_REGIONS) == null)  return null;
+      return Splitter.on(",").trimResults().split((CharSequence) providerMetadata.getDefaultProperties().get(LocationConstants.PROPERTY_REGIONS));
    }
 
    private String getResourceGroupFromId(String id) {

If we have a global property that filters locations, we should also filter the `listNodes` operation.

How should the `getImage` and `getNode` behave when invoked with a resource that is from another location?

>        if (region != null) {
          builder.iso3166Codes(ImmutableSet.of(region.iso3166Code()));
       }
-
       return builder.build();
    }
 

Now that we agree we shouldn't change REGION to ZONE, let's discard the changes in this class.

> @@ -46,9 +44,9 @@ protected void bindErrorHandlers() {
    @Override
    protected void installLocations() {
       super.installLocations();
-      bind(ImplicitLocationSupplier.class).
-              to(OnlyLocationOrFirstRegionOptionallyMatchingRegionId.class).

This was working before. If you discard the changes in the location transformation function, can't you just discard the changes in this class too?

>  
 import static com.google.common.base.Preconditions.checkNotNull;
-import org.jclouds.logging.slf4j.config.SLF4JLoggingModule;
-import org.jclouds.logging.config.LoggingModule;
-
+import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.RESOURCE_GROUP_NAME;
+import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_SCRIPT_COMPLETE;
+import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_NODE_RUNNING;
+import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_PORT_OPEN;
+import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_NODE_TERMINATED;
+import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_NODE_SUSPENDED;
 
 /**
  * Live tests for the {@link org.jclouds.compute.ComputeService} integration.

Just remove this from the change-set

> @@ -28,6 +30,7 @@ public static Properties defaultProperties(Properties properties) {
        properties.put("oauth.credential", "password");
        properties.put("oauth.endpoint", "https://login.microsoftonline.com/oauth2/token");
        properties.put(CREDENTIAL_TYPE, CLIENT_CREDENTIALS_SECRET.toString());
+       properties.put(PROPERTY_REGIONS, "northeurope,eastus");

When rolling back the location transformation function you'll probably have to change these values. They also appear in the README as an example, so bear in mind to change that too.

-- 
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/308#pullrequestreview-520038

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.



> @@ -300,28 +288,43 @@ public VMImage getImage(final String id) {
 
    @Override
    public Iterable<Location> listLocations() {
+      final Iterable<String> whiteListZoneName = findWhiteListOfRegions();

Also, better process this in the constructor and assign as a final variable of this class.

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

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
>     @Inject
>     AzureComputeServiceAdapter(final AzureComputeApi api, final AzureComputeConstants azureComputeConstants,
> -                              CleanupResources cleanupResources) {
> +                              CleanupResources cleanupResources, ProviderMetadata providerMetadata) {

yes

-- 
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/308/files/961db5c145342971726063e25776e0011ab8ed36#r74558750

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
@nacx good to merge?

-- 
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/308#issuecomment-246959604

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Andrea Turli <no...@github.com>.
@andreaturli pushed 1 commit.

07007d9  refactor logback-test.xml


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/308/files/8f403aa5cc1c711496a670c3fbf3e03eadca969e..07007d994a33b1cc3c62a6294e8760141014758e

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.



> @@ -28,6 +30,7 @@ public static Properties defaultProperties(Properties properties) {
        properties.put("oauth.credential", "password");
        properties.put("oauth.endpoint", "https://login.microsoftonline.com/oauth2/token");
        properties.put(CREDENTIAL_TYPE, CLIENT_CREDENTIALS_SECRET.toString());
+       properties.put(PROPERTY_REGIONS, "northeurope,eastus");

Ok, dismiss this comment. I was under the assumption that the changes in the LocationToLocation class were made to support the ZONE approach, but I see they're not related. What is the purpose of that change in the location id ?

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

Re: [jclouds/jclouds-labs] add support for whitelisting locations (#308)

Posted by Ignasi Barrera <no...@github.com>.
> @@ -28,6 +30,7 @@ public static Properties defaultProperties(Properties properties) {
>         properties.put("oauth.credential", "password");
>         properties.put("oauth.endpoint", "https://login.microsoftonline.com/oauth2/token");
>         properties.put(CREDENTIAL_TYPE, CLIENT_CREDENTIALS_SECRET.toString());
> +       properties.put(PROPERTY_ZONES, "northeurope,eastus");

How do locations look like in Azure? These looks more like Regions? I mean, zones in providers are usually in the form: northeurope-1, northeurope2, etc.

-- 
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/308/files/961db5c145342971726063e25776e0011ab8ed36#r74422920