nacx commented on this pull request.
Thanks for a small but important PR @trevorflanagan!
> + private static final String REDHAT = "RedHat";
+ private static final String UBUNTU = "Ubuntu";
+ private static final String SUSE = "SuSE";
+ private static final String WINDOWS = "Win";
+
+ @Override
+ public OsFamily apply(final String description) {
+ if (description != null) {
+ if (description.contains(CENTOS))
+ return OsFamily.CENTOS;
+ else if (description.contains(UBUNTU))
+ return OsFamily.UBUNTU;
+ else if (description.contains(REDHAT))
+ return OsFamily.RHEL;
+ else if (description.contains(SUSE))
+ return OsFamily.RHEL;
Shouldn't it be `OsFamily.SUSE`?
> + OsImageToImage(@Memoized final Supplier<Set<Location>> locations,
+ Function<String, OsFamily> imageDescriptionToOsFamily) {
+ this.locations = locations;
+ this.imageDescriptionToOsFamily = imageDescriptionToOsFamily;
+ }
+
+ @Override
+ public Image apply(OsImage input) {
+
+ OsFamily osFamily =
imageDescriptionToOsFamily.apply(input.description());
+ String osVersion = parseVersion(input.description());
+
+ OperatingSystem os =
OperatingSystem.builder().name(input.name()).description(input.description())
+
.family(osFamily).version(osVersion).is64Bit(is64bit(input)).build();
+
+ return new
ImageBuilder().id(input.id()).name(input.name()).description(input.description())
When configuring the image IDs, think about the "id" and the "providerId" field
this way:
* **providerId** is the *real id* of the image in the provider (I guess
`input.id()`), so users can retrieve the image using the provider-specific APIs.
* **id** is the *portable id jclouds will use to uniquelly identify the image*.
This is slightly different from the previous one. For example, in some
providers, images in different regions may have the same id (centos-6,
ami-0001, etc), but jcouds needs to provide unique IDs for each, so we encode
the region in the id (something like: "region/id").
I don't know if it is the case in DimensionData, but please consider this when
configuring the id and providerId fields. If the ID is unique, then just assign
the same value to both fields :)
> + this.locations = locations;
+ this.imageDescriptionToOsFamily = imageDescriptionToOsFamily;
+ }
+
+ @Override
+ public Image apply(OsImage input) {
+
+ OsFamily osFamily =
imageDescriptionToOsFamily.apply(input.description());
+ String osVersion = parseVersion(input.description());
+
+ OperatingSystem os =
OperatingSystem.builder().name(input.name()).description(input.description())
+
.family(osFamily).version(osVersion).is64Bit(is64bit(input)).build();
+
+ return new
ImageBuilder().id(input.id()).name(input.name()).description(input.description())
+ .status(Image.Status.AVAILABLE).operatingSystem(os).location(
+
FluentIterable.from(locations.get()).firstMatch(LocationPredicates.idEquals(input.datacenterId()))
Just one consideration about the meaning of the `location` field in an image:
* If the image is globally available in all locations (the same image, same id,
etc), then the location field should be `null`.
* if the image is only available in certain locations, then the location field
must be set accordingly.
> +
+ @BeforeMethod
+ public void setUp() throws Exception {
+ imageDescriptionToOsFamily = new ImageDescriptionToOsFamily();
+ }
+
+ @Test
+ public void apply_Centos() {
+ String description = "CentOS 6.0 (Final)";
+ assertEquals(OsFamily.CENTOS,
imageDescriptionToOsFamily.apply(description));
+ }
+
+ @Test
+ public void apply_Suse() {
+ String description = "SuSE Linux Enterprise Server 11 SP4";
+ assertEquals(OsFamily.RHEL,
imageDescriptionToOsFamily.apply(description));
`OsFamily.SUSE`?
> + *
+ * 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.dimensiondata.cloudcontrol.compute.functions;
+
+import org.jclouds.compute.domain.OsFamily;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import static org.testng.AssertJUnit.assertEquals;
+
+public class ImageDescriptionToOsFamilyTest {
Configure the test in the "unit" group.
--
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/421#pullrequestreview-79411168
