You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Carlos García Ibáñez <no...@github.com> on 2013/10/28 16:35:29 UTC
[jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
The new method checks if the volume is in Attached state and, if so, retrieves the corresponding Virtual Machine. Otherwise, it just returns null.
You can merge this Pull Request by running:
git pull https://github.com/carlosgarciaibanez/jclouds-labs master
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs/pull/31
-- Commit Summary --
* Added a method to retrieve the Virtual Machine a Volume is attached to
-- File Changes --
M abiquo/src/main/java/org/jclouds/abiquo/domain/cloud/Volume.java (23)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/31.patch
https://github.com/jclouds/jclouds-labs/pull/31.diff
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Andrew Phillips <no...@github.com>.
> jclouds » jclouds-labs #530 SUCCESS
BuildHive looks happier now ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27239105
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Andrew Phillips <no...@github.com>.
+1 - looks good to me. Thanks, @carlosgarciaibanez!
@nacx: OK with you?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27707631
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #73](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/73/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27671941
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Andrew Phillips <no...@github.com>.
> + /**
> + * Retrieve the virtual machine this volume is attached to.
> + *
> + * @return The virtual machine this volume is attached to, or null if it is
> + * not attached.
> + */
> + public VirtualMachine getVirtualMachine() {
> + checkState(ATTACHED == VolumeState.valueOf(target.getState()), "Volume is not attached to a VM");
> + RESTLink vmLink = checkNotNull(target.searchLink(ParentLinkName.VIRTUAL_MACHINE),
> + ValidationErrors.MISSING_REQUIRED_LINK + " " + ParentLinkName.VIRTUAL_MACHINE);
> + vmLink.setType(VirtualMachineWithNodeExtendedDto.BASE_MEDIA_TYPE);
> + HttpResponse response = context.getApi().get(vmLink);
> +
> + ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto> parser = new ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto>(
> + context.utils().xml(),
> + TypeLiteral.get(VirtualMachineWithNodeExtendedDto.class));
Still not clear why we are explicitly instantiating a parser here. This should be injected by Guice if we need it, no? But can't we make a different API call that gives us back a VirtualMachine directly?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31/files#r7280889
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #71](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/71/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27282349
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #530](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/530/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27231768
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Andrew Phillips <no...@github.com>.
> + /**
> + * Retrieve the virtual machine this volume is attached to.
> + *
> + * @return The virtual machine this volume is attached to, or null if it is
> + * not attached.
> + */
> + public VirtualMachine getVirtualMachine() {
> + checkState(ATTACHED == VolumeState.valueOf(target.getState()), "Volume is not attached to a VM");
> + RESTLink vmLink = checkNotNull(target.searchLink(ParentLinkName.VIRTUAL_MACHINE),
> + ValidationErrors.MISSING_REQUIRED_LINK + " " + ParentLinkName.VIRTUAL_MACHINE);
> + vmLink.setType(VirtualMachineWithNodeExtendedDto.BASE_MEDIA_TYPE);
> + HttpResponse response = context.getApi().get(vmLink);
> +
> + ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto> parser = new ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto>(
> + context.utils().xml(),
> + TypeLiteral.get(VirtualMachineWithNodeExtendedDto.class));
> Personally, I think it is cleaner to use the method to get a generic link and manually call the parser to parse the result, than parsing
> the URI of the link to call a concrete method of the specific api class
Makes sense to me...and certainly I'm not the expert on the Abiquo API who could have a firm opinion here ;-) But getting the parser from the injector rather than trying to construct it ourselves certainly seems like an improvement.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31/files#r7361221
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Ignasi Barrera <no...@github.com>.
Merged. Thanks @demobox and @carlosgarciaibanez!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27753909
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #550](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/550/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27752601
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Carlos García Ibáñez <no...@github.com>.
The last commit includes the changes suggested by @nacx and @demobox.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27282459
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Ignasi Barrera <no...@github.com>.
> + /**
> + * Retrieve the virtual machine this volume is attached to.
> + *
> + * @return The virtual machine this volume is attached to, or null if it is
> + * not attached.
> + */
> + public VirtualMachine getVirtualMachine() {
> + checkState(ATTACHED == VolumeState.valueOf(target.getState()), "Volume is not attached to a VM");
> + RESTLink vmLink = checkNotNull(target.searchLink(ParentLinkName.VIRTUAL_MACHINE),
> + ValidationErrors.MISSING_REQUIRED_LINK + " " + ParentLinkName.VIRTUAL_MACHINE);
> + vmLink.setType(VirtualMachineWithNodeExtendedDto.BASE_MEDIA_TYPE);
> + HttpResponse response = context.getApi().get(vmLink);
> +
> + ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto> parser = new ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto>(
> + context.utils().xml(),
> + TypeLiteral.get(VirtualMachineWithNodeExtendedDto.class));
Agree. We could request the parser to the Guice injector by providing the appropriate type token. I think that would be a cleaner approach.
Regarding the specific API call, I think it wouldn't provide benefit: To be able to make that call we would have to parse the URI in the link to extract the different ids to be passed as a parameter to the api call. This adds the complexity, and the chance to introduce a bug, in the parsing logic.
It is also worth taking into account that the Abiquo Api does a strong use of the HATEOAS concept, and if you take a look at the api classes you con't see any endpoint (almost any) declared there. This is because the endpoints are extracted from the objects returned by the Api. This has the advantages that if, for any reason, a URI changed, jclouds would continue working as all endpoints are generated dinamically. If we manually parse the URI in the link, we will lose this.
Apart from that, calling the generic method in the AbiquoApi to get the link, only implies one manual step: parsing the result. But we already know the type of the result and can ask Guice for the appropriate parser, so that shouldn't be a problem.
Personally, I think it is cleaner to use the method to get a generic link and manually call the parser to parse the result, than parsing the URI of the link to call a concrete method of the specific api class. (I'm open to discuss this though :D).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31/files#r7324342
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #72](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/72/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27282606
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Ignasi Barrera <no...@github.com>.
> + * @return The virtual machine this volume is attached to, or null if it is
> + * not attached.
> + */
> + public VirtualMachine getVirtualMachine() {
> + if ( ATTACHED == valueOf(target.getState()) ) {
> + RESTLink vmLink = checkNotNull(target.searchLink(ParentLinkName.VIRTUAL_MACHINE),
> + ValidationErrors.MISSING_REQUIRED_LINK + " " + ParentLinkName.VIRTUAL_MACHINE);
> + vmLink.setType(VirtualMachineWithNodeExtendedDto.BASE_MEDIA_TYPE);
> + HttpResponse response = context.getApi().get(vmLink);
> +
> + ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto> parser = new ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto>(
> + context.utils().xml(),
> + TypeLiteral.get(VirtualMachineWithNodeExtendedDto.class));
> + return wrap(context, VirtualMachine.class, parser.apply(response));
> + }
> + return null;
In jclouds we try to avoid returning `null`, and we rely on Guava's `Optional` instead (see [UsingAndAvoidingNullExplained](http://code.google.com/p/guava-libraries/wiki/UsingAndAvoidingNullExplained)). In this concrete case, however, I think it should be better to change the method to fail if the volume is not attached (callers should take care of checking that before calling this method), by replacing the first conditional by something like:
```java
checkState(ATTACHED == valueOf(target.getState()), "volume is not attached to a vm");
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31/files#r7259189
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Andrew Phillips <no...@github.com>.
> @@ -90,6 +95,28 @@ public VirtualDatacenter getVirtualDatacenter() {
> return virtualDatacenter;
> }
>
> + /**
> + * Retrieve the virtual machine this volume is attached to, or null if it is
> + * detached.
> + *
> + * @return The virtual machine this volume is attached to, or null if it is
> + * not attached.
> + */
> + public VirtualMachine getVirtualMachine() {
> + if ( ATTACHED == valueOf(target.getState()) ) {
[formatting] Remove spaces inside the `(...)`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31/files#r7264958
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Andrew Phillips <no...@github.com>.
> + * Retrieve the virtual machine this volume is attached to, or null if it is
> + * detached.
> + *
> + * @return The virtual machine this volume is attached to, or null if it is
> + * not attached.
> + */
> + public VirtualMachine getVirtualMachine() {
> + if ( ATTACHED == valueOf(target.getState()) ) {
> + RESTLink vmLink = checkNotNull(target.searchLink(ParentLinkName.VIRTUAL_MACHINE),
> + ValidationErrors.MISSING_REQUIRED_LINK + " " + ParentLinkName.VIRTUAL_MACHINE);
> + vmLink.setType(VirtualMachineWithNodeExtendedDto.BASE_MEDIA_TYPE);
> + HttpResponse response = context.getApi().get(vmLink);
> +
> + ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto> parser = new ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto>(
> + context.utils().xml(),
> + TypeLiteral.get(VirtualMachineWithNodeExtendedDto.class));
Is there any way we can make a "proper" API call here, rather than making a raw HTTP call as we do here and apply all the parsing etc. ourselves?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31/files#r7264949
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Andrew Phillips <no...@github.com>.
> + * Retrieve the virtual machine this volume is attached to.
> + *
> + * @return The virtual machine this volume is attached to, or null if it is
> + * not attached.
> + */
> + public VirtualMachine getVirtualMachine() {
> + checkState(ATTACHED == VolumeState.valueOf(target.getState()), "Volume is not attached to a VM");
> + RESTLink vmLink = checkNotNull(target.searchLink(ParentLinkName.VIRTUAL_MACHINE),
> + ValidationErrors.MISSING_REQUIRED_LINK + " " + ParentLinkName.VIRTUAL_MACHINE);
> + vmLink.setType(VirtualMachineWithNodeExtendedDto.BASE_MEDIA_TYPE);
> + HttpResponse response = context.getApi().get(vmLink);
> +
> + ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto> parser = context.utils()
> + .injector()
> + .getInstance(
> + Key.get(new TypeLiteral<ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto>>(){}));
> I'll make some improvements tomorrow morning if you want.
If you have time, that would be nice. But if @nacx has already merged by then, also fine ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31/files#r7409656
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #529](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/529/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27226282
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Ignasi Barrera <no...@github.com>.
> + /**
> + * Retrieve the virtual machine this volume is attached to.
> + *
> + * @return The virtual machine this volume is attached to, or null if it is
> + * not attached.
> + */
> + public VirtualMachine getVirtualMachine() {
> + checkState(ATTACHED == VolumeState.valueOf(target.getState()), "Volume is not attached to a VM");
> + RESTLink vmLink = checkNotNull(target.searchLink(ParentLinkName.VIRTUAL_MACHINE),
> + ValidationErrors.MISSING_REQUIRED_LINK + " " + ParentLinkName.VIRTUAL_MACHINE);
> + vmLink.setType(VirtualMachineWithNodeExtendedDto.BASE_MEDIA_TYPE);
> + HttpResponse response = context.getApi().get(vmLink);
> +
> + ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto> parser = new ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto>(
> + context.utils().xml(),
> + TypeLiteral.get(VirtualMachineWithNodeExtendedDto.class));
@demobox your comments are very welcome!
In this case it is not about the Abiquo API itself, but about choosing between manually parsing the link to extract the fields to build a concrete api call, or to use the generic method and manually call the response parser. Both make concessions and have pros and cons. It is just a matter of preference and pick the one we are more comfortable with :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31/files#r7387263
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #67](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/67/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27223906
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #74](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/74/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27752392
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #68](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/68/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27224148
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Andrew Phillips <no...@github.com>.
> jclouds » jclouds-labs #527 FAILURE
Looks like a [real compilation error](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/527/console). Could you have a look at this, @carlosgarciaibanez? Thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27223442
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #69](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/69/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27224964
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #532](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/532/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27282961
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Andrew Phillips <no...@github.com>.
> + * Retrieve the virtual machine this volume is attached to.
> + *
> + * @return The virtual machine this volume is attached to, or null if it is
> + * not attached.
> + */
> + public VirtualMachine getVirtualMachine() {
> + checkState(ATTACHED == VolumeState.valueOf(target.getState()), "Volume is not attached to a VM");
> + RESTLink vmLink = checkNotNull(target.searchLink(ParentLinkName.VIRTUAL_MACHINE),
> + ValidationErrors.MISSING_REQUIRED_LINK + " " + ParentLinkName.VIRTUAL_MACHINE);
> + vmLink.setType(VirtualMachineWithNodeExtendedDto.BASE_MEDIA_TYPE);
> + HttpResponse response = context.getApi().get(vmLink);
> +
> + ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto> parser = context.utils()
> + .injector()
> + .getInstance(
> + Key.get(new TypeLiteral<ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto>>(){}));
[minor] Odd formatting?
```
ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto> parser = context.utils().injector()
.getInstance(Key.get(new TypeLiteral<ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto>>(){}));
```
or so? But also find to proceed without this.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31/files#r7407663
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Carlos García Ibáñez <no...@github.com>.
If BuildHive is happy, I'm happy :-)
El 28/10/2013 19:10, "Andrew Phillips" <no...@github.com> escribió:
> jclouds » jclouds-labs #530 SUCCESS
>
> BuildHive looks happier now ;-)
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27239105>
> .
>
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27240379
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Andrew Phillips <no...@github.com>.
Let me close'n'reopen this as I think BuildHive might have got a bit confused...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27230434
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #528](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/528/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27224590
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #527](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/527/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27223240
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Ignasi Barrera <no...@github.com>.
> + * Retrieve the virtual machine this volume is attached to, or null if it is
> + * detached.
> + *
> + * @return The virtual machine this volume is attached to, or null if it is
> + * not attached.
> + */
> + public VirtualMachine getVirtualMachine() {
> + if ( ATTACHED == valueOf(target.getState()) ) {
> + RESTLink vmLink = checkNotNull(target.searchLink(ParentLinkName.VIRTUAL_MACHINE),
> + ValidationErrors.MISSING_REQUIRED_LINK + " " + ParentLinkName.VIRTUAL_MACHINE);
> + vmLink.setType(VirtualMachineWithNodeExtendedDto.BASE_MEDIA_TYPE);
> + HttpResponse response = context.getApi().get(vmLink);
> +
> + ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto> parser = new ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto>(
> + context.utils().xml(),
> + TypeLiteral.get(VirtualMachineWithNodeExtendedDto.class));
I think it is safe to do this (see discussion in the mailing list [here](http://markmail.org/message/d76czl4tjm4vsjmi)). This is not performing a raw http call, but using a [generic method in the AbiquoApi](https://github.com/jclouds/jclouds-labs/blob/master/abiquo/src/main/java/org/jclouds/abiquo/AbiquoApi.java#L119-L122) to get the resource pointed by the link.
That api call will be managed as other methods in the delegating apis, so all error handling, retry logic, etc, will also apply to this call. The only thing that is being done manually is to parse the output if the request succeeds, but we already know here which is the returned type, so I think it is safe to do this here.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31/files#r7272763
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Ignasi Barrera <no...@github.com>.
> + /**
> + * Retrieve the virtual machine this volume is attached to.
> + *
> + * @return The virtual machine this volume is attached to, or null if it is
> + * not attached.
> + */
> + public VirtualMachine getVirtualMachine() {
> + checkState(ATTACHED == VolumeState.valueOf(target.getState()), "Volume is not attached to a VM");
> + RESTLink vmLink = checkNotNull(target.searchLink(ParentLinkName.VIRTUAL_MACHINE),
> + ValidationErrors.MISSING_REQUIRED_LINK + " " + ParentLinkName.VIRTUAL_MACHINE);
> + vmLink.setType(VirtualMachineWithNodeExtendedDto.BASE_MEDIA_TYPE);
> + HttpResponse response = context.getApi().get(vmLink);
> +
> + ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto> parser = new ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto>(
> + context.utils().xml(),
> + TypeLiteral.get(VirtualMachineWithNodeExtendedDto.class));
Something like (haven't tested it, but should work):
```java
ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto> parser =
context.utils().injector().getInstance(Key.get(
new TypeLiteral<ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto>>() {
}
));
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31/files#r7395290
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Carlos García Ibáñez <no...@github.com>.
> + * Retrieve the virtual machine this volume is attached to.
> + *
> + * @return The virtual machine this volume is attached to, or null if it is
> + * not attached.
> + */
> + public VirtualMachine getVirtualMachine() {
> + checkState(ATTACHED == VolumeState.valueOf(target.getState()), "Volume is not attached to a VM");
> + RESTLink vmLink = checkNotNull(target.searchLink(ParentLinkName.VIRTUAL_MACHINE),
> + ValidationErrors.MISSING_REQUIRED_LINK + " " + ParentLinkName.VIRTUAL_MACHINE);
> + vmLink.setType(VirtualMachineWithNodeExtendedDto.BASE_MEDIA_TYPE);
> + HttpResponse response = context.getApi().get(vmLink);
> +
> + ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto> parser = context.utils()
> + .injector()
> + .getInstance(
> + Key.get(new TypeLiteral<ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto>>(){}));
I agree with you: it's a piece of odd formatted code, I'll make some
improvements tomorrow morning if you want.
El 04/11/2013 19:04, "Andrew Phillips" <no...@github.com> escribió:
> In abiquo/src/main/java/org/jclouds/abiquo/domain/cloud/Volume.java:
>
> > + * Retrieve the virtual machine this volume is attached to.
> > + *
> > + * @return The virtual machine this volume is attached to, or null if it is
> > + * not attached.
> > + */
> > + public VirtualMachine getVirtualMachine() {
> > + checkState(ATTACHED == VolumeState.valueOf(target.getState()), "Volume is not attached to a VM");
> > + RESTLink vmLink = checkNotNull(target.searchLink(ParentLinkName.VIRTUAL_MACHINE),
> > + ValidationErrors.MISSING_REQUIRED_LINK + " " + ParentLinkName.VIRTUAL_MACHINE);
> > + vmLink.setType(VirtualMachineWithNodeExtendedDto.BASE_MEDIA_TYPE);
> > + HttpResponse response = context.getApi().get(vmLink);
> > +
> > + ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto> parser = context.utils()
> > + .injector()
> > + .getInstance(
> > + Key.get(new TypeLiteral<ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto>>(){}));
>
> [minor] Odd formatting?
>
> ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto> parser = context.utils().injector()
> .getInstance(Key.get(new TypeLiteral<ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto>>(){}));
>
> or so? But also find to proceed without this.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/jclouds/jclouds-labs/pull/31/files#r7407663>
> .
>
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31/files#r7409133
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Carlos García Ibáñez <no...@github.com>.
> + /**
> + * Retrieve the virtual machine this volume is attached to.
> + *
> + * @return The virtual machine this volume is attached to, or null if it is
> + * not attached.
> + */
> + public VirtualMachine getVirtualMachine() {
> + checkState(ATTACHED == VolumeState.valueOf(target.getState()), "Volume is not attached to a VM");
> + RESTLink vmLink = checkNotNull(target.searchLink(ParentLinkName.VIRTUAL_MACHINE),
> + ValidationErrors.MISSING_REQUIRED_LINK + " " + ParentLinkName.VIRTUAL_MACHINE);
> + vmLink.setType(VirtualMachineWithNodeExtendedDto.BASE_MEDIA_TYPE);
> + HttpResponse response = context.getApi().get(vmLink);
> +
> + ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto> parser = new ParseXMLWithJAXB<VirtualMachineWithNodeExtendedDto>(
> + context.utils().xml(),
> + TypeLiteral.get(VirtualMachineWithNodeExtendedDto.class));
Hi, guys. Could you point me to an example of how to get the proper injected parser? I'm looking around the code, but I'm not quite sure about how to do it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31/files#r7395243
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #70](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/70/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27230944
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by Carlos García Ibáñez <no...@github.com>.
Sorry, I forgot to include a class in the commit. I hope it is fixed now.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27223718
Re: [jclouds-labs] Added a method to retrieve the Virtual Machine a
Volume is attached to (#31)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #66](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/66/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/31#issuecomment-27222808