You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Iván Lomba <no...@github.com> on 2016/07/18 22:39:04 UTC
[jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case (#984)
- ArbitraryCpuRamTemplateBuilderImpl and GoogleComputeEngineArbitraryCpuRamTemplateBuilderImpl subclasses.
- Parse utility.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/984
-- Commit Summary --
* First approach to ArbitraryCpuRamTemplateBuilderImpl
* Several fixes: refactoring some names, format, identation problems, some missing license headers and generateId method
* Refactored parse utility
* Added GoogleComputeEngineArbitraryCpuRamTemplateBuilderImpl to support GCE custom machine URI
* extracted hardware creation to automaticHardwareForCpuAndRam method
* Fixed ide automatic asterisk imports
-- File Changes --
M compute/src/main/java/org/jclouds/compute/config/BaseComputeServiceContextModule.java (60)
A compute/src/main/java/org/jclouds/compute/domain/internal/ArbitraryCpuRamTemplateBuilderImpl.java (82)
M compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java (84)
A compute/src/main/java/org/jclouds/compute/util/AutomaticHardwareIdSpec.java (59)
A compute/src/test/java/org/jclouds/compute/domain/internal/ArbitraryCpuRamTemplateBuilderImplTest.java (142)
A compute/src/test/java/org/jclouds/compute/util/AutomaticHardwareIdSpecTest.java (51)
M providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/config/GoogleComputeEngineServiceContextModule.java (60)
A providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/domain/internal/GoogleComputeEngineArbitraryCpuRamTemplateBuilderImpl.java (58)
A providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/domain/internal/GoogleComputeEngineArbitraryCpuRamTemplateBuilderImplTest.java (153)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/984.patch
https://github.com/jclouds/jclouds/pull/984.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/pull/984
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -91,6 +91,8 @@ protected void configure() {
> bind(new TypeLiteral<ComputeServiceAdapter<Instance, MachineType, Image, Location>>() {
> }).to(GoogleComputeEngineServiceAdapter.class);
>
> + bind(ArbitraryCpuRamTemplateBuilderImpl.class).to(GoogleComputeEngineArbitraryCpuRamTemplateBuilderImpl.class);
This should be:
```java
bind(TemplateBuilder.class).to(GoogleComputeEngineArbitraryCpuRamTemplateBuilderImpl.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/pull/984/files/24d1c709310ffe7e511e13c203f2d7f0d4709d37#r71836317
Re: [jclouds/jclouds] JCLOUDS-482: ArbitraryCpuRam base case (#984)
Posted by Iván Lomba <no...@github.com>.
> + .trimResults()
> + .omitEmptyStrings()
> + .withKeyValueSeparator('=')
> + .split(hardwareSpec);
> + if (!specValues.containsKey("ram") || !specValues.containsKey("cores")) {
> + throw new IllegalArgumentException(String.format("Omitted keys on hardwareId: %s. Please set number " +
> + "of cores and ram amount.", hardwareId));
> + }
> + spec.ram = Integer.parseInt(specValues.get("ram"));
> + spec.cores = Double.parseDouble(specValues.get("cores"));
> + return spec;
> + }
> +
> + public static AutomaticHardwareIdSpec automaticHardwareIdSpecBuilder(double cores, int ram) {
> + AutomaticHardwareIdSpec spec = new AutomaticHardwareIdSpec();
> + if (cores == 0 || ram == 0) {
Yes, It's only checking that the user has specified both values (they are primitives).
---
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/pull/984/files/55c696c2ba870e202cfb8e42734e66bb31aab7b4#r73974500
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> + Provider<TemplateBuilder> templateBuilderProvider = createMock(Provider.class);
> + TemplateBuilder defaultTemplate = createMock(TemplateBuilder.class);
> + TemplateOptions defaultOptions = createMock(TemplateOptions.class);
> + GetImageStrategy getImageStrategy = createMock(GetImageStrategy.class);
> +
> + expect(optionsProvider.get()).andReturn(new TemplateOptions());
> + expect(getImageStrategy.getImage(anyObject(String.class))).andReturn(null);
> + replay(defaultTemplate, optionsProvider, templateBuilderProvider, getImageStrategy);
> + TemplateBuilderImpl templateBuilder = new GoogleComputeEngineArbitraryCpuRamTemplateBuilderImpl(locations, new ImageCacheSupplier(images, 60,
> + Atomics.<AuthorizationException>newReference(), Providers.of(getImageStrategy)), hardwares,
> + Suppliers.ofInstance(region), optionsProvider, templateBuilderProvider);
> + templateBuilder.minRam(1024);
> + templateBuilder.minCores(2);
> + Hardware hardware = templateBuilder.build().getHardware();
> + assertThat(hardware.getRam()).isEqualTo(1024);
> + assertThat(hardware.getProcessors()).extracting("cores").containsExactly(2.0);
Verify the hardware ID too, to make sure an automatic one has been picked.
---
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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71279882
Re: [jclouds/jclouds] JCLOUDS-482: ArbitraryCpuRam base case (#984)
Posted by Andrew Phillips <no...@github.com>.
> + @Named("DEFAULT") Provider<TemplateBuilder> defaultTemplateProvider) {
> + super(locations, images, hardwares, defaultLocation, optionsProvider, defaultTemplateProvider);
> + }
> +
> + protected Hardware automaticHardwareForCpuAndRam(double cores, int ram) {
> + return new HardwareBuilder()
> + .id(automaticHardwareIdSpecBuilder(cores, ram).toString())
> + .ram(ram)
> + .processor(new Processor(cores, 1.0))
> + .build();
> + }
> +
> + protected Hardware findHardwareWithId(Set<? extends Hardware> hardwaresToSearch) {
> + try {
> + return super.findHardwareWithId(hardwaresToSearch);
> + } catch (NoSuchElementException ex) {
Is catching an exception and falling back the best way of doing this? Would the following alternative perhaps also work?
```
if (isAutomaticId(...)) {
// do stuff for automated IDs
} else {
// fall back to original behaviour
}
```
The immediate challenge I can see with this is that users _might_ have existing hardware IDs that match this scheme. Is there a way we can avoid this by not "overloading" the ID in some way?
---
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/pull/984/files/55c696c2ba870e202cfb8e42734e66bb31aab7b4#r73571672
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> + if (isAutomaticId(hardwareId)) {
> + AutomaticHardwareIdSpec spec = parseId(hardwareId);
> + return automaticHardwareForCpuAndRam(spec.getCores(), spec.getRam());
> + }
> + else{
> + throw throwNoSuchElementExceptionAfterLoggingHardwareIds("Invalid hardwareId", hardwaresToSearch);
> + }
> + }
> + }
> +
> + protected Hardware resolveHardware(Set<? extends Hardware> hardwarel, final Iterable<? extends Image> images) {
> + try {
> + return super.resolveHardware(hardwarel, images);
> + }
> + catch (Exception NoSuchElementException) {
> + if (super.minCores != 0 && super.minRam != 0) {
Another option would be to throw a NoSuchElementException with a proper messsge. Something like "No hardware profile was found matching the given criteria. If you want to use exact values, please set the minCores and minRam values".
---
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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71649033
Re: [jclouds/jclouds] JCLOUDS-482: ArbitraryCpuRam base case (#984)
Posted by Andrew Phillips <no...@github.com>.
> + .build();
> + }
> +
> + protected Hardware findHardwareWithId(Set<? extends Hardware> hardwaresToSearch) {
> + try {
> + return super.findHardwareWithId(hardwaresToSearch);
> + } catch (NoSuchElementException ex) {
> + if (isAutomaticId(hardwareId)) {
> + AutomaticHardwareIdSpec spec = parseId(hardwareId);
> + return automaticHardwareForCpuAndRam(spec.getCores(), spec.getRam());
> + }
> + else {
> + throw ex;
> + }
> + }
> + }
[minor] Indenting in the above section is off?
---
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/pull/984/files/55c696c2ba870e202cfb8e42734e66bb31aab7b4#r73571087
Re: [jclouds/jclouds] JCLOUDS-482: ArbitraryCpuRam base case (#984)
Posted by Andrew Phillips <no...@github.com>.
> + .trimResults()
> + .omitEmptyStrings()
> + .withKeyValueSeparator('=')
> + .split(hardwareSpec);
> + if (!specValues.containsKey("ram") || !specValues.containsKey("cores")) {
> + throw new IllegalArgumentException(String.format("Omitted keys on hardwareId: %s. Please set number " +
> + "of cores and ram amount.", hardwareId));
> + }
> + spec.ram = Integer.parseInt(specValues.get("ram"));
> + spec.cores = Double.parseDouble(specValues.get("cores"));
> + return spec;
> + }
> +
> + public static AutomaticHardwareIdSpec automaticHardwareIdSpecBuilder(double cores, int ram) {
> + AutomaticHardwareIdSpec spec = new AutomaticHardwareIdSpec();
> + if (cores == 0 || ram == 0) {
Are negative values allowed?
---
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/pull/984/files/55c696c2ba870e202cfb8e42734e66bb31aab7b4#r73572349
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.compute.domain.*;
> +import org.jclouds.compute.options.TemplateOptions;
> +import org.jclouds.compute.strategy.GetImageStrategy;
> +import org.jclouds.compute.suppliers.ImageCacheSupplier;
> +import org.jclouds.domain.Location;
> +import org.jclouds.domain.LocationBuilder;
> +import org.jclouds.domain.LocationScope;
> +import org.jclouds.rest.AuthorizationException;
> +import org.testng.annotations.Test;
> +
> +import java.util.Set;
> +
> +import static org.assertj.core.api.Assertions.assertThat;
> +import static org.easymock.EasyMock.*;
> +
> +@Test(groups = "unit", singleThreaded = true, testName = "TemplateBuilderImplTest")
Fix the test 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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71279168
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Iván Lomba <no...@github.com>.
@ivanlomba pushed 1 commit.
5cd7685 Arbitrary hardware tests added to BaseTemplateBuilderLiveTest and GoogleComputeEngineTemplateBuilderLiveTest
---
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/984/files/dc05f567b1400b08027a33a58988e3b121f8c3d1..5cd7685b7eb9a5a23d1549fa5018231f6872d2e2
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> + TemplateBuilder defaultTemplate = createMock(TemplateBuilder.class);
> + TemplateOptions defaultOptions = createMock(TemplateOptions.class);
> + GetImageStrategy getImageStrategy = createMock(GetImageStrategy.class);
> +
> + expect(optionsProvider.get()).andReturn(new TemplateOptions());
> + expect(getImageStrategy.getImage(anyObject(String.class))).andReturn(null);
> + replay(defaultTemplate, optionsProvider, templateBuilderProvider, getImageStrategy);
> + TemplateBuilderImpl templateBuilder = new ArbitraryCpuRamTemplateBuilderImpl(locations, new ImageCacheSupplier(images, 60,
> + Atomics.<AuthorizationException>newReference(), Providers.of(getImageStrategy)), hardwares,
> + Suppliers.ofInstance(region), optionsProvider, templateBuilderProvider);
> + templateBuilder.minRam(1024);
> + templateBuilder.minCores(2);
> + Hardware hardware = templateBuilder.build().getHardware();
> + assertThat(hardware.getRam()).isEqualTo(1024);
> + assertThat(hardware.getProcessors()).extracting("cores").containsExactly(2.0);
> + }
Add a test to verify what happens when you only set the minRam or the minCores, as it is a path that your class is explicitly handling.
Also add a couple more tests where there is actually a hardware profile present:
* One test where the existing hardware profile does not match, and an automatic one is used.
* One test where the existing hardware profile matches the given cpu and ram, to verify that the existing hardware profile is 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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71279447
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> + }
> + else{
> + throw throwNoSuchElementExceptionAfterLoggingHardwareIds("Invalid hardwareId", hardwaresToSearch);
> + }
> + }
> + }
> +
> + protected Hardware resolveHardware(Set<? extends Hardware> hardwarel, final Iterable<? extends Image> images) {
> + try {
> + return super.resolveHardware(hardwarel, images);
> + }
> + catch (Exception NoSuchElementException) {
> + if (super.minCores != 0 && super.minRam != 0) {
> + return automaticHardwareForCpuAndRam(minCores, minRam);
> + }
> + else throw throwNoSuchElementExceptionAfterLoggingHardwareIds("Invalid hardware", hardwarel);
Same here. Consider just propagating the current 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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71278830
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> + AutomaticHardwareIdSpec spec = new AutomaticHardwareIdSpec();
> + String hardwareSpec = hardwareId.substring(10);
> + Map<String, String> specValues = Splitter.on(';')
> + .trimResults()
> + .omitEmptyStrings()
> + .withKeyValueSeparator('=')
> + .split(hardwareSpec);
> + if (!specValues.containsKey("ram") || !specValues.containsKey("cores")) {
> + throw new IllegalArgumentException(String.format("Omitted keys on hardwareId: %s", hardwareId));
> + }
> + spec.ram = Integer.parseInt(specValues.get("ram"));
> + spec.cores = Double.parseDouble(specValues.get("cores"));
> + return spec;
> + }
> +
> + public static AutomaticHardwareIdSpec AutomaticHardwareIdSpecBuilder(Double cores, int ram) {
Why one argument is an object t and the other is a primitive type?
---
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/pull/984/files/24d1c709310ffe7e511e13c203f2d7f0d4709d37#r71836014
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> + private Integer ram;
> +
> + public static boolean isAutomaticId(String id) {
> + return id.startsWith("automatic:");
> + }
> +
> + public static AutomaticHardwareIdSpec parseId(String hardwareId) {
> + AutomaticHardwareIdSpec spec = new AutomaticHardwareIdSpec();
> + String hardwareSpec = hardwareId.substring(10);
> + Map<String, String> specValues = Splitter.on(';')
> + .trimResults()
> + .omitEmptyStrings()
> + .withKeyValueSeparator('=')
> + .split(hardwareSpec);
> + if (!specValues.containsKey("ram") || !specValues.containsKey("cores")) {
> + throw new IllegalArgumentException(String.format("Omitted keys on hardwareId: %s", hardwareId));
Give feedback about what is missing.
---
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/pull/984/files/24d1c709310ffe7e511e13c203f2d7f0d4709d37#r71835996
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @ivanlomba! Good start. Apart from the inline comments, have a look at the formatting, and re-indent the code to use a 3 space indent. Also have a look at the checkstyle violations.
---
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/pull/984#issuecomment-233545912
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> + Map<String, String> specValues = Splitter.on(';')
> + .trimResults()
> + .omitEmptyStrings()
> + .withKeyValueSeparator('=')
> + .split(hardwareSpec);
> + if (!specValues.containsKey("ram") || !specValues.containsKey("cores")) {
> + throw new IllegalArgumentException(String.format("Omitted keys on hardwareId: %s", hardwareId));
> + }
> + spec.ram = Integer.parseInt(specValues.get("ram"));
> + spec.cores = Double.parseDouble(specValues.get("cores"));
> + return spec;
> + }
> +
> + public static String generateAutomaticId(double cores, int ram) {
> + return String.format("automatic:cores=%s;ram=%s", cores, ram);
> + }
Consider overriding the `toString()` method instead of having this 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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71279090
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -91,6 +92,7 @@ protected void configure() {
> }).to(CreateSshClientOncePortIsListeningOnNode.class);
> bind(new TypeLiteral<Function<TemplateOptions, Statement>>() {
> }).to(TemplateOptionsToStatement.class);
> + bind(TemplateBuilderImpl.class).to(ArbitraryCpuRamTemplateBuilderImpl.class);
This cannot be done here. This binding must be applied in the module of each provider that supports an arbitrary cpu and ram config.
---
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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71278378
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> + TemplateBuilder defaultTemplate = createMock(TemplateBuilder.class);
> + TemplateOptions defaultOptions = createMock(TemplateOptions.class);
> + GetImageStrategy getImageStrategy = createMock(GetImageStrategy.class);
> +
> + expect(optionsProvider.get()).andReturn(new TemplateOptions());
> + expect(getImageStrategy.getImage(anyObject(String.class))).andReturn(null);
> + replay(defaultTemplate, optionsProvider, templateBuilderProvider, getImageStrategy);
> + TemplateBuilderImpl templateBuilder = new ArbitraryCpuRamTemplateBuilderImpl(locations, new ImageCacheSupplier(images, 60,
> + Atomics.<AuthorizationException>newReference(), Providers.of(getImageStrategy)), hardwares,
> + Suppliers.ofInstance(region), optionsProvider, templateBuilderProvider);
> + templateBuilder.minRam(1024);
> + templateBuilder.minCores(2);
> + Hardware hardware = templateBuilder.build().getHardware();
> + assertThat(hardware.getRam()).isEqualTo(1024);
> + assertThat(hardware.getProcessors()).extracting("cores").containsExactly(2.0);
> + }
Also verify the hardware ID too, to make sure an automatic one has been picked.
---
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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71279930
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
This looks pretty good @ivanlomba! 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/pull/984#issuecomment-236544789
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> + .id(generateAutomaticId(cores, ram))
> + .ram(ram)
> + .processor(new Processor(cores, 1.0))
> + .build();
> + }
> +
> + protected Hardware findHardwareWithId(Set<? extends Hardware> hardwaresToSearch) {
> + try {
> + return super.findHardwareWithId(hardwaresToSearch);
> + } catch (Exception NoSuchElementException) {
> + if (isAutomaticId(hardwareId)) {
> + AutomaticHardwareIdSpec spec = parseId(hardwareId);
> + return automaticHardwareForCpuAndRam(spec.getCores(), spec.getRam());
> + }
> + else{
> + throw throwNoSuchElementExceptionAfterLoggingHardwareIds("Invalid hardwareId", hardwaresToSearch);
Consider just propagating the current exception 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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71278753
Re: [jclouds/jclouds] JCLOUDS-482: ArbitraryCpuRam base case (#984)
Posted by Andrew Phillips <no...@github.com>.
> + } catch (NoSuchElementException ex) {
> + if (isAutomaticId(hardwareId)) {
> + AutomaticHardwareIdSpec spec = parseId(hardwareId);
> + return automaticHardwareForCpuAndRam(spec.getCores(), spec.getRam());
> + }
> + else {
> + throw ex;
> + }
> + }
> + }
> +
> + protected Hardware resolveHardware(Set<? extends Hardware> hardwarel, final Iterable<? extends Image> images) {
> + try {
> + return super.resolveHardware(hardwarel, images);
> + }
> + catch (NoSuchElementException ex) {
Same comment as above - can we avoid "try then catch" as a strategy 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/pull/984/files/55c696c2ba870e202cfb8e42734e66bb31aab7b4#r73571836
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
The build is failing due to checkstyle violations
>[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:2.15:check (default) on project jclouds-compute: You have 2 Checkstyle violations. -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:2.15:check (default) on project jclouds-compute: You have 2 Checkstyle violations.
You can run a local build, and have a look at the `compute/target/checkstyle-result.xml` for the details.
---
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/pull/984#issuecomment-234473682
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> -import com.google.inject.TypeLiteral;
> -import com.google.inject.assistedinject.FactoryModuleBuilder;
> -import com.google.inject.name.Names;
> +import javax.inject.Named;
> +import javax.inject.Singleton;
> +import java.util.Map;
> +import java.util.Set;
> +import java.util.concurrent.Callable;
> +import java.util.concurrent.TimeUnit;
> +import java.util.concurrent.atomic.AtomicReference;
> +
> +import static com.google.common.base.Preconditions.checkNotNull;
> +import static org.jclouds.Constants.PROPERTY_SESSION_INTERVAL;
> +import static org.jclouds.compute.config.ComputeServiceProperties.IMAGE_ID;
> +import static org.jclouds.compute.config.ComputeServiceProperties.TEMPLATE;
> +import static org.jclouds.compute.domain.OsFamily.UBUNTU;
>
> public abstract class BaseComputeServiceContextModule extends AbstractModule {
>
Remove changes in this class; it is only reorganizing the imports.
---
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/pull/984/files/24d1c709310ffe7e511e13c203f2d7f0d4709d37#r71835908
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> + else{
> + throw ex;
> + }
> + }
> + }
> +
> + protected Hardware resolveHardware(Set<? extends Hardware> hardwarel, final Iterable<? extends Image> images) {
> + try {
> + return super.resolveHardware(hardwarel, images);
> + }
> + catch (NoSuchElementException ex) {
> + if (super.minCores != 0 && super.minRam != 0) {
> + return automaticHardwareForCpuAndRam(minCores, minRam);
> + }
> + else throw new NoSuchElementException("No hardware profile matching the given criteria was found. " +
> + "If you want to use exact values, please set the minCores and minRam values");
Wrap the original exception as the cause of this one, to keep the complete stacktrace.
---
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/pull/984/files/24d1c709310ffe7e511e13c203f2d7f0d4709d37#r71835855
Re: [jclouds/jclouds] JCLOUDS-482: ArbitraryCpuRam base case (#984)
Posted by Andrew Phillips <no...@github.com>.
> +import org.jclouds.domain.LocationBuilder;
> +import org.jclouds.domain.LocationScope;
> +import org.jclouds.rest.AuthorizationException;
> +import org.testng.annotations.Test;
> +
> +import java.net.URI;
> +import java.util.Set;
> +
> +import static org.assertj.core.api.Assertions.assertThat;
> +import static org.easymock.EasyMock.anyObject;
> +import static org.easymock.EasyMock.createMock;
> +import static org.easymock.EasyMock.expect;
> +import static org.easymock.EasyMock.replay;
> +
> +@Test(groups = "unit", singleThreaded = true, testName = "ArbitraryCpuRamTemplateBuilderImplTest")
> +public class ArbitraryCpuRamTemplateBuilderImplTest {
Can we extend another test and inherit the fixture?
---
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/pull/984/files/55c696c2ba870e202cfb8e42734e66bb31aab7b4#r73572472
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + super(locations, images, hardwares, defaultLocation, optionsProvider, defaultTemplateProvider);
> + }
> +
> + protected Hardware automaticHardwareForCpuAndRam(double cores, int ram) {
> + return new HardwareBuilder()
> + .id(generateAutomaticId(cores, ram))
> + .ram(ram)
> + .processor(new Processor(cores, 1.0))
> + .build();
> + }
> +
> + protected Hardware findHardwareWithId(Set<? extends Hardware> hardwaresToSearch) {
> + try {
> + return super.findHardwareWithId(hardwaresToSearch);
> + } catch (Exception NoSuchElementException) {
Variable names should start in lower case.
---
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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71278520
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + super(locations, images, hardwares, defaultLocation, optionsProvider, defaultTemplateProvider);
> + }
> +
> + protected Hardware automaticHardwareForCpuAndRam(double cores, int ram) {
> + return new HardwareBuilder()
> + .id(generateAutomaticId(cores, ram))
> + .ram(ram)
> + .processor(new Processor(cores, 1.0))
> + .build();
> + }
> +
> + protected Hardware findHardwareWithId(Set<? extends Hardware> hardwaresToSearch) {
> + try {
> + return super.findHardwareWithId(hardwaresToSearch);
> + } catch (Exception NoSuchElementException) {
Also, just `catch (NoSuchElementException ex)`
---
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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71278585
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Iván Lomba <no...@github.com>.
@ivanlomba pushed 2 commits.
1576198 Move repeated constants to TestUtils to reuse code
55c696c Fix full path in the Hardware id and URI
---
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/984/files/9fb02ff7a884ee2d21aa55ee07c1d23a0ebf358a..55c696c2ba870e202cfb8e42734e66bb31aab7b4
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Iván Lomba <no...@github.com>.
@ivanlomba pushed 1 commit.
9fb02ff Added two more tests to BaseTemplateBuilderLiveTest
---
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/984/files/5cd7685b7eb9a5a23d1549fa5018231f6872d2e2..9fb02ff7a884ee2d21aa55ee07c1d23a0ebf358a
Re: [jclouds/jclouds] JCLOUDS-482: ArbitraryCpuRam base case (#984)
Posted by Ignasi Barrera <no...@github.com>.
Squashed, rebased to the latest version of master and merged to the GSoC branch. I've also cherry-picked the commit in https://github.com/jclouds/jclouds/pull/993.
--
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/pull/984#issuecomment-239022448
Re: [jclouds/jclouds] JCLOUDS-482: ArbitraryCpuRam base case (#984)
Posted by Iván Lomba <no...@github.com>.
@ivanlomba pushed 3 commits.
5c7d50e Add custom hardware tests to BaseComputeServiceLiveTest and GCEServiceLiveTest
95db047 Cherry-pick: Always take into account the configured template builder spec in live tests
ea5ae64 Change customHardware test to use buildTemplate and fix identation
---
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/984/files/55c696c2ba870e202cfb8e42734e66bb31aab7b4..ea5ae64af7517f90bfd9cbdc0a850aee65127150
Re: [jclouds/jclouds] JCLOUDS-482: ArbitraryCpuRam base case (#984)
Posted by Andrew Phillips <no...@github.com>.
> +package org.jclouds.compute.util;
> +
> +import com.google.common.base.Splitter;
> +
> +import java.util.Map;
> +
> +public class AutomaticHardwareIdSpec {
> +
> + private double cores;
> + private int ram;
> +
> + public static boolean isAutomaticId(String id) {
> + return id.startsWith("automatic:");
> + }
> +
> + public static AutomaticHardwareIdSpec parseId(String hardwareId) {
I'm guessing there's no way to avoid encoding this a string and using an (automatically deserialized) object instead?
---
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/pull/984/files/55c696c2ba870e202cfb8e42734e66bb31aab7b4#r73572227
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> + AutomaticHardwareIdSpec spec = new AutomaticHardwareIdSpec();
> + String hardwareSpec = hardwareId.substring(10);
> + Map<String, String> specValues = Splitter.on(';')
> + .trimResults()
> + .omitEmptyStrings()
> + .withKeyValueSeparator('=')
> + .split(hardwareSpec);
> + if (!specValues.containsKey("ram") || !specValues.containsKey("cores")) {
> + throw new IllegalArgumentException(String.format("Omitted keys on hardwareId: %s", hardwareId));
> + }
> + spec.ram = Integer.parseInt(specValues.get("ram"));
> + spec.cores = Double.parseDouble(specValues.get("cores"));
> + return spec;
> + }
> +
> + public static AutomaticHardwareIdSpec AutomaticHardwareIdSpecBuilder(Double cores, int ram) {
Also, method names should start with a lower case letter.
---
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/pull/984/files/24d1c709310ffe7e511e13c203f2d7f0d4709d37#r71836068
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Iván Lomba <no...@github.com>.
> @@ -131,6 +129,7 @@
> @VisibleForTesting
> protected Boolean forceCacheReload;
>
> + @VisibleForTesting
Yes, it was a misprint, I don't know how this annotation came 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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71595157
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> + } catch (Exception NoSuchElementException) {
> + if (isAutomaticId(hardwareId)) {
> + AutomaticHardwareIdSpec spec = parseId(hardwareId);
> + return automaticHardwareForCpuAndRam(spec.getCores(), spec.getRam());
> + }
> + else{
> + throw throwNoSuchElementExceptionAfterLoggingHardwareIds("Invalid hardwareId", hardwaresToSearch);
> + }
> + }
> + }
> +
> + protected Hardware resolveHardware(Set<? extends Hardware> hardwarel, final Iterable<? extends Image> images) {
> + try {
> + return super.resolveHardware(hardwarel, images);
> + }
> + catch (Exception NoSuchElementException) {
Same 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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71278633
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.jclouds.compute.util;
> +
> +import com.google.common.base.Splitter;
> +
> +import java.util.Map;
> +
> +public class AutomaticHardwareIdSpec {
> +
> + private Double cores;
> + private Integer ram;
If both fields are mandatory and an instance of this class can't be built without both being set, change the types to primitives to enforce non-null values.
---
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/pull/984/files/24d1c709310ffe7e511e13c203f2d7f0d4709d37#r71836128
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Iván Lomba <no...@github.com>.
@ivanlomba pushed 1 commit.
bf61477 correcting WIP base case PR according to comments
---
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d..bf614772d70e1236f266421605dc1542655d78a2
Re: [jclouds/jclouds] JCLOUDS-482: ArbitraryCpuRam base case (#984)
Posted by Andrew Phillips <no...@github.com>.
Thanks for all the work on this, @ivanlomba! My main question is about the need to encode the "arbitrary spec" in a string and do our our parsing, rather than somehow using an object and having jclouds do the (de-)serialization automatically.
If that's already something that's been discussed on the mailing list of elsewhere, apologies ;-)
/cc @ivanlomba
---
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/pull/984#issuecomment-237634409
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -44,6 +49,9 @@
> @Test(groups = "integration,live")
> public abstract class BaseTemplateBuilderLiveTest extends BaseComputeServiceContextLiveTest {
>
> + public static final Object[][] NO_INVOCATIONS = new Object[0][0];
> + public static final Object[][] SINGLE_NO_ARG_INVOCATION = { new Object[0] };
This is a recurring pattern. Move these constants to the [TestUtils](https://github.com/jclouds/jclouds/blob/master/core/src/test/java/org/jclouds/utils/TestUtils.java) class, and also remove them from the filesystem provider and other classes, so all data providers using this pattern use the same constants.
---
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/pull/984/files/9fb02ff7a884ee2d21aa55ee07c1d23a0ebf358a#r72955188
Re: [jclouds/jclouds] JCLOUDS-482: ArbitraryCpuRam base case (#984)
Posted by Ignasi Barrera <no...@github.com>.
Closed #984.
--
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/pull/984#event-752008149
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> +import com.google.inject.Provider;
> +import com.google.inject.util.Providers;
> +import org.jclouds.compute.domain.*;
> +import org.jclouds.compute.options.TemplateOptions;
> +import org.jclouds.compute.strategy.GetImageStrategy;
> +import org.jclouds.compute.suppliers.ImageCacheSupplier;
> +import org.jclouds.domain.Location;
> +import org.jclouds.domain.LocationBuilder;
> +import org.jclouds.domain.LocationScope;
> +import org.jclouds.rest.AuthorizationException;
> +import org.testng.annotations.Test;
> +
> +import java.util.Set;
> +
> +import static org.assertj.core.api.Assertions.assertThat;
> +import static org.easymock.EasyMock.*;
Remove wildcard imports
---
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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71279179
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> + if (isAutomaticId(hardwareId)) {
> + AutomaticHardwareIdSpec spec = parseId(hardwareId);
> + return automaticHardwareForCpuAndRam(spec.getCores(), spec.getRam());
> + }
> + else{
> + throw throwNoSuchElementExceptionAfterLoggingHardwareIds("Invalid hardwareId", hardwaresToSearch);
> + }
> + }
> + }
> +
> + protected Hardware resolveHardware(Set<? extends Hardware> hardwarel, final Iterable<? extends Image> images) {
> + try {
> + return super.resolveHardware(hardwarel, images);
> + }
> + catch (Exception NoSuchElementException) {
> + if (super.minCores != 0 && super.minRam != 0) {
As commented in the commit, should allow users to just pass the cpu or the ram? Should we have some defaults for that case?
---
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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71278803
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Iván Lomba <no...@github.com>.
> + if (isAutomaticId(hardwareId)) {
> + AutomaticHardwareIdSpec spec = parseId(hardwareId);
> + return automaticHardwareForCpuAndRam(spec.getCores(), spec.getRam());
> + }
> + else{
> + throw throwNoSuchElementExceptionAfterLoggingHardwareIds("Invalid hardwareId", hardwaresToSearch);
> + }
> + }
> + }
> +
> + protected Hardware resolveHardware(Set<? extends Hardware> hardwarel, final Iterable<? extends Image> images) {
> + try {
> + return super.resolveHardware(hardwarel, images);
> + }
> + catch (Exception NoSuchElementException) {
> + if (super.minCores != 0 && super.minRam != 0) {
Considering that if only minRam or minCores are provided and they match with a hardware profile a VM is created according to the hardware profile, I think that we should have some defaults. I will ask on the mail list about the default values.
---
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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71595038
Re: [jclouds/jclouds] JCLOUDS-482: ArbitraryCpuRam base case (#984)
Posted by Andrew Phillips <no...@github.com>.
> @@ -727,7 +724,7 @@ private Image loadImageWithId(Iterable<? extends Image> images) {
> return image.get();
> }
>
> - private Hardware findHardwareWithId(Set<? extends Hardware> hardwaresToSearch) {
> + protected Hardware findHardwareWithId(Set<? extends Hardware> hardwaresToSearch) {
> Hardware hardware;
> // TODO: switch to GetHardwareStrategy in version 1.5
Likely unrelated to this commit, but...is this comment still relevant?
---
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/pull/984/files/55c696c2ba870e202cfb8e42734e66bb31aab7b4#r73571947
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Iván Lomba <no...@github.com>.
@ivanlomba pushed 1 commit.
24d1c70 added machineTypeUriToHardware to set custom hardware in nodes
---
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/984/files/bf614772d70e1236f266421605dc1542655d78a2..24d1c709310ffe7e511e13c203f2d7f0d4709d37
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Iván Lomba <no...@github.com>.
@ivanlomba pushed 2 commits.
cff580e fix checkstyle violations and other PR comments
dc05f56 Set the providerId to custom machineType URI and fix adding node log
---
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/984/files/24d1c709310ffe7e511e13c203f2d7f0d4709d37..dc05f567b1400b08027a33a58988e3b121f8c3d1
Re: [jclouds/jclouds] JCLOUDS-482: ArbitraryCpuRam base case (#984)
Posted by Iván Lomba <no...@github.com>.
> + .trimResults()
> + .omitEmptyStrings()
> + .withKeyValueSeparator('=')
> + .split(hardwareSpec);
> + if (!specValues.containsKey("ram") || !specValues.containsKey("cores")) {
> + throw new IllegalArgumentException(String.format("Omitted keys on hardwareId: %s. Please set number " +
> + "of cores and ram amount.", hardwareId));
> + }
> + spec.ram = Integer.parseInt(specValues.get("ram"));
> + spec.cores = Double.parseDouble(specValues.get("cores"));
> + return spec;
> + }
> +
> + public static AutomaticHardwareIdSpec automaticHardwareIdSpecBuilder(double cores, int ram) {
> + AutomaticHardwareIdSpec spec = new AutomaticHardwareIdSpec();
> + if (cores == 0 || ram == 0) {
Thanks @demobox ! :)
--
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/pull/984/files/55c696c2ba870e202cfb8e42734e66bb31aab7b4#r74689785
Re: [jclouds/jclouds] JCLOUDS-482: ArbitraryCpuRam base case (#984)
Posted by Andrew Phillips <no...@github.com>.
> + }
> +
> + public static AutomaticHardwareIdSpec parseId(String hardwareId) {
> + AutomaticHardwareIdSpec spec = new AutomaticHardwareIdSpec();
> + String hardwareSpec = hardwareId.substring(10);
> + Map<String, String> specValues = Splitter.on(';')
> + .trimResults()
> + .omitEmptyStrings()
> + .withKeyValueSeparator('=')
> + .split(hardwareSpec);
> + if (!specValues.containsKey("ram") || !specValues.containsKey("cores")) {
> + throw new IllegalArgumentException(String.format("Omitted keys on hardwareId: %s. Please set number " +
> + "of cores and ram amount.", hardwareId));
> + }
> + spec.ram = Integer.parseInt(specValues.get("ram"));
> + spec.cores = Double.parseDouble(specValues.get("cores"));
Do we need some exception handling 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/pull/984/files/55c696c2ba870e202cfb8e42734e66bb31aab7b4#r73572289
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> + .trimResults()
> + .omitEmptyStrings()
> + .withKeyValueSeparator('=')
> + .split(hardwareSpec);
> + if (!specValues.containsKey("ram") || !specValues.containsKey("cores")) {
> + throw new IllegalArgumentException(String.format("Omitted keys on hardwareId: %s", hardwareId));
> + }
> + spec.ram = Integer.parseInt(specValues.get("ram"));
> + spec.cores = Double.parseDouble(specValues.get("cores"));
> + return spec;
> + }
> +
> + public static AutomaticHardwareIdSpec AutomaticHardwareIdSpecBuilder(Double cores, int ram) {
> + AutomaticHardwareIdSpec spec = new AutomaticHardwareIdSpec();
> + if (cores == 0 || ram == 0) {
> + throw new IllegalArgumentException(String.format("Omitted or wrong parms"));
Be explicit about what is exactly missing or 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/pull/984/files/24d1c709310ffe7e511e13c203f2d7f0d4709d37#r71836041
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -131,6 +129,7 @@
> @VisibleForTesting
> protected Boolean forceCacheReload;
>
> + @VisibleForTesting
It is not just visible for testing; it is visible to allow inheritance, so 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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71278873
Re: [jclouds/jclouds] JCLOUDS-482: ArbitraryCpuRam base case (#984)
Posted by Andrew Phillips <no...@github.com>.
> + .trimResults()
> + .omitEmptyStrings()
> + .withKeyValueSeparator('=')
> + .split(hardwareSpec);
> + if (!specValues.containsKey("ram") || !specValues.containsKey("cores")) {
> + throw new IllegalArgumentException(String.format("Omitted keys on hardwareId: %s. Please set number " +
> + "of cores and ram amount.", hardwareId));
> + }
> + spec.ram = Integer.parseInt(specValues.get("ram"));
> + spec.cores = Double.parseDouble(specValues.get("cores"));
> + return spec;
> + }
> +
> + public static AutomaticHardwareIdSpec automaticHardwareIdSpecBuilder(double cores, int ram) {
> + AutomaticHardwareIdSpec spec = new AutomaticHardwareIdSpec();
> + if (cores == 0 || ram == 0) {
> It's only checking that the user has specified both values (they are primitives).
What would we get here if one of the values was *not* specified? Wouldn't we see the same default value here? Also, equality checking of floating-point values is a little tricky, because depending on the system you can get things like `a/2 - a/2 != 0` due to rounding issues.
---
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/pull/984/files/55c696c2ba870e202cfb8e42734e66bb31aab7b4#r74047558
Re: [jclouds/jclouds] JCLOUDS-482: WIP ArbitraryCpuRam base case
(#984)
Posted by Ignasi Barrera <no...@github.com>.
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.jclouds.compute.util;
> +
> +import org.testng.annotations.Test;
> +
> +import static org.assertj.core.api.Assertions.assertThat;
> +
> +@Test(groups = "unit", testName = "IdParserTest")
Fix test 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/pull/984/files/5e38477f80df5dd0dce54f7773cf9e883fbf3d2d#r71279477