aledsage commented on this pull request.
LGTM - only a couple of very minor comments.
> + VirtualMachineProperties oldProperties = vm.properties();
+ StorageProfile oldStorageProfile = oldProperties.storageProfile();
+
+ String blob =
storageService.storageServiceProperties().primaryEndpoints().get("blob");
+ VHD vhd = VHD.create(blob + "vhds/" + vmName + "new-data-disk.vhd");
+ DataDisk newDataDisk = DataDisk.create(vmName + "new-data-disk", "1", 1,
vhd, "Empty");
+ List<DataDisk> oldDataDisks = oldStorageProfile.dataDisks();
+
+ ImmutableList<DataDisk> newDataDisks = ImmutableList.<DataDisk>
builder().addAll(oldDataDisks).add(newDataDisk).build();
+ StorageProfile newStorageProfile =
oldStorageProfile.toBuilder().dataDisks(newDataDisks).build();
+ VirtualMachineProperties newProperties =
oldProperties.toBuilder().storageProfile(newStorageProfile).build();
+
+ VirtualMachine newVm = vm.toBuilder().properties(newProperties).build();
+ vm = api().createOrUpdate(vmName, newVm.location(), newVm.properties(),
newVm.tags(), newVm.plan());
+
+ assertEquals(vm.properties().storageProfile().dataDisks().size(), 2);
Perhaps assert it's equal to `oldDataDisks.size() + 1` (or assert earlier that
the oldDataDisks size is 1)?
> @@ -16,11 +16,6 @@
*/
package org.jclouds.azurecompute.arm.features;
-import static org.assertj.core.api.Assertions.assertThat;
Would be nice if the PR didn't move the imports around, but no strong feelings
from me.
> @@ -88,11 +88,11 @@
@Path("/{vmname}")
@QueryParams(keys = "validating", values = "false")
@Produces(MediaType.APPLICATION_JSON)
- VirtualMachine create(@PathParam("vmname") String vmname,
- @PayloadParam("location") String location,
- @PayloadParam("properties") VirtualMachineProperties
properties,
- @PayloadParam("tags") Map<String, String> tags,
- @Nullable @PayloadParam("plan") Plan plan);
+ VirtualMachine createOrUpdate(@PathParam("vmname") String vmname,
Could change javadoc of this method as well (not that the javadoc on most of
these methods is particularly helpful, e.g. `start()`'s javadoc says "The start
Virtual Machine 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/368#pullrequestreview-24868205
