Re: [jclouds/jclouds-labs] add compute functions (#346)

[Andrea Turli]

Closed #346.

-- 
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/346#event-936315783

Re: [jclouds/jclouds-labs] add compute functions (#346)

[Andrea Turli]

merged at 
[master](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/a91ff3bd)

-- 
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/346#issuecomment-275139640

Re: [jclouds/jclouds-labs] add compute functions (#346)

[Ignasi Barrera]

nacx approved this pull request.





-- 
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/346#pullrequestreview-18416045

Re: [jclouds/jclouds-labs] add compute functions (#346)

[Andrea Turli]

andreaturli commented on this pull request.



> +
+private final JustProvider justProvider;
+
+// allow us to lazy discover the provider of a resource
+@Inject
+FacilityToLocation(JustProvider justProvider) {
+this.justProvider = justProvider;
+}
+
+@Override
+public Location apply(final Facility facility) {
+final LocationBuilder builder = new LocationBuilder();
+builder.id(facility.code());
+builder.description(facility.name());
+builder.parent(getOnlyElement(justProvider.get()));
+builder.scope(LocationScope.REGION);

I think the docs is not in sync: there is no `state` in the live json object :(

-- 
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/346

Re: [jclouds/jclouds-labs] add compute functions (#346)

[Ignasi Barrera]

nacx commented on this pull request.



> +
+private final JustProvider justProvider;
+
+// allow us to lazy discover the provider of a resource
+@Inject
+FacilityToLocation(JustProvider justProvider) {
+this.justProvider = justProvider;
+}
+
+@Override
+public Location apply(final Facility facility) {
+final LocationBuilder builder = new LocationBuilder();
+builder.id(facility.code());
+builder.description(facility.name());
+builder.parent(getOnlyElement(justProvider.get()));
+builder.scope(LocationScope.REGION);

I was referring to the `state` field (according to their docs). Are there new 
fields that have been added recently on their side?
https://www.packet.net/developers/api/devices/#devices-locations

-- 
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/346

Re: [jclouds/jclouds-labs] add compute functions (#346)

[Andrea Turli]

andreaturli commented on this pull request.



> +
+private final JustProvider justProvider;
+
+// allow us to lazy discover the provider of a resource
+@Inject
+FacilityToLocation(JustProvider justProvider) {
+this.justProvider = justProvider;
+}
+
+@Override
+public Location apply(final Facility facility) {
+final LocationBuilder builder = new LocationBuilder();
+builder.id(facility.code());
+builder.description(facility.name());
+builder.parent(getOnlyElement(justProvider.get()));
+builder.scope(LocationScope.REGION);

it'd be a bit tricky as the API returns 
https://github.com/jclouds/jclouds-labs/blob/master/packet/src/test/resources/facilities.json
 and it is not straightforward to get the correct ISO-3166 from the `name` 
and/or `code` I think

-- 
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/346

Re: [jclouds/jclouds-labs] add compute functions (#346)

[Ignasi Barrera]

nacx commented on this pull request.



> +
+@Resource
+@Named(ComputeServiceConstants.COMPUTE_LOGGER)
+protected Logger logger = Logger.NULL;
+
+private final PlanToHardware planToHardware;
+private final OperatingSystemToImage operatingSystemToImage;
+private final FacilityToLocation facilityToLocation;
+private final Function toPortableStatus;
+private final GroupNamingConvention groupNamingConvention;
+
+@Inject
+DeviceToNodeMetadata(PlanToHardware planToHardware, OperatingSystemToImage 
operatingSystemToImage, FacilityToLocation facilityToLocation,
+ Function 
toPortableStatus,
+ GroupNamingConvention.Factory groupNamingConvention) {
+this.planToHardware = checkNotNull(planToHardware, "planToHardware 
cannot be null");

No need for null checks in the injection constructors.

> +
+private final JustProvider justProvider;
+
+// allow us to lazy discover the provider of a resource
+@Inject
+FacilityToLocation(JustProvider justProvider) {
+this.justProvider = justProvider;
+}
+
+@Override
+public Location apply(final Facility facility) {
+final LocationBuilder builder = new LocationBuilder();
+builder.id(facility.code());
+builder.description(facility.name());
+builder.parent(getOnlyElement(justProvider.get()));
+builder.scope(LocationScope.REGION);

Can the ISO-3166 code be inferred in a consistent way, for example from the 
`state` field?

> +public class PlanToHardware implements Function {
+
+@Override
+public Hardware apply(Plan plan) {
+HardwareBuilder builder = new HardwareBuilder()
+.ids(plan.slug())
+.name(plan.name())
+.hypervisor("none")
+.processors(getProcessors(plan))
+.ram(getMemory(plan))
+.volumes(getVolumes(plan));
+return builder.build();
+}
+
+private Integer getMemory(Plan plan) {
+if (plan.specs() == null || plan.specs().drives() == null) return -1;

Better return 0 ?

-- 
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/346#pullrequestreview-18362581

Re: [jclouds/jclouds-labs] add compute functions (#346)

[Andrea Turli]

@andreaturli pushed 1 commit.

3825f9a  improve DeviceToNodeMetadata


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/346/files/d1074295696a9bac88a3a6e9bb1c7581e295ae40..3825f9aea216c127fb948e586a2291c0177af269