andreaturli commented on this pull request.
lgtm, some minor comments
> @@ -114,7 +103,7 @@ public static Properties defaultProperties() {
properties.put(API_VERSION_PREFIX +
PublicIPAddressApi.class.getSimpleName(), "2015-06-15");
properties.put(API_VERSION_PREFIX +
ResourceGroupApi.class.getSimpleName(), "2015-01-01");
properties.put(API_VERSION_PREFIX +
ResourceProviderApi.class.getSimpleName(), "2015-01-01");
- properties.put(API_VERSION_PREFIX +
StorageAccountApi.class.getSimpleName(), STORAGE_API_VERSION);
+ properties.put(API_VERSION_PREFIX +
StorageAccountApi.class.getSimpleName(), "2015-06-15");
we should decide at some point if we want to use the latest version of all of
them or just use them randomly :D
> }
- public void deleteResourceGroupIfEmpty(String group) {
- if (api.getVirtualMachineApi(group).list().isEmpty()
- && api.getStorageAccountApi(group).list().isEmpty()
+ public boolean deleteResourceGroupIfEmpty(String group) {
+ boolean deleted = false;
+ if (api.getVirtualMachineApi(group).list().isEmpty() &&
api.getStorageAccountApi(group).list().isEmpty()
those api calls are really expensive, I'll try to replace them in a subsequent
PR
> RegionAndId regionAndId = RegionAndId.fromSlashEncoded(id);
- String group = locationToResourceGroupName.apply(regionAndId.region());
-
+ ResourceGroup resourceGroup =
resourceGroupMap.getUnchecked(regionAndId.region());
+ String group = resourceGroup.name();
[minor] could you name it `resourceGroupName`
> final RegionAndId regionAndId =
> RegionAndId.fromSlashEncoded(cloneTemplate.getSourceNodeId());
- final String group =
locationToResourceGroupName.apply(regionAndId.region());
+ ResourceGroup resourceGroup =
resourceGroupMap.getUnchecked(regionAndId.region());
+ final String group = resourceGroup.name();
[minor] maybe `resourceGroupName`?
> + AzureComputeSecurityGroupExtension(AzureComputeApi api, @Memoized
> Supplier<Set<? extends Location>> locations,
+ Function<NetworkSecurityGroup, SecurityGroup> groupConverter,
+ SecurityGroupAvailablePredicateFactory securityRuleAvailable,
+ @Named(TIMEOUT_RESOURCE_DELETED) Predicate<URI> resourceDeleted,
+ LoadingCache<String, ResourceGroup> resourceGroupMap) {
+ this.api = api;
+ this.locations = locations;
+ this.securityGroupConverter = groupConverter;
+ this.securityGroupAvailable = securityRuleAvailable;
+ this.resourceDeleted = resourceDeleted;
+ this.resourceGroupMap = resourceGroupMap;
+ }
+
+ @Override
+ public Set<SecurityGroup> listSecurityGroups() {
+ return ImmutableSet.copyOf(concat(transform(locations.get(), new
Function<Location, Set<SecurityGroup>>() {
why immutableSet?
> + VirtualMachine vm =
> api.getVirtualMachineApi(resourceGroup.name()).get(regionAndId.id());
+ List<IdReference> networkInterfacesIdReferences =
vm.properties().networkProfile().networkInterfaces();
+ List<NetworkSecurityGroup> networkGroups = new
ArrayList<NetworkSecurityGroup>();
+
+ for (IdReference networkInterfaceCardIdReference :
networkInterfacesIdReferences) {
+ String nicName =
Iterables.getLast(Splitter.on("/").split(networkInterfaceCardIdReference.id()));
+ NetworkInterfaceCard card =
api.getNetworkInterfaceCardApi(resourceGroup.name()).get(nicName);
+ if (card != null && card.properties().networkSecurityGroup() != null)
{
+ String secGroupName = Iterables.getLast(Splitter.on("/").split(
+ card.properties().networkSecurityGroup().id()));
+ NetworkSecurityGroup group =
api.getNetworkSecurityGroupApi(resourceGroup.name()).get(secGroupName);
+ networkGroups.add(group);
+ }
+ }
+
+ return ImmutableSet.copyOf(transform(filter(networkGroups, notNull()),
securityGroupConverter));
why immutableSet?
> + final RegionAndId regionAndId =
> RegionAndId.fromSlashEncoded(group.getId());
+ ResourceGroup resourceGroup =
resourceGroupMap.getUnchecked(regionAndId.region());
+
+ NetworkSecurityGroupApi groupApi =
api.getNetworkSecurityGroupApi(resourceGroup.name());
+ NetworkSecurityGroup networkSecurityGroup =
groupApi.get(regionAndId.id());
+
+ if (networkSecurityGroup == null) {
+ throw new IllegalArgumentException("Security group " +
group.getName() + " was not found");
+ }
+
+ NetworkSecurityRuleApi ruleApi =
api.getNetworkSecurityRuleApi(resourceGroup.name(),
networkSecurityGroup.name());
+ int nextPriority = getRuleStartingPriority(networkSecurityGroup);
+
+ for (String ipRange : ipRanges) {
+ NetworkSecurityRuleProperties properties =
NetworkSecurityRuleProperties.builder()
+ .protocol(Protocol.fromValue(protocol.name())) //
please 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/347#pullrequestreview-18640274
