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
