Re: [DISCUSS] Moving to Java 8
On 16-07-15 17:58, John Burwell wrote: Wido, We have an acute problem — Oracle will be issuing no further security patches for Java7 which is a significant opsec risk. Put simply, we can’t leave our users exposed to such a risk because Ubuntu won’t ship a release for an non-EOL’ed Java version for another year. Personally, I think it is a tremendous oversight that Java7 has been EOL’ed for over three (3) months [1], and we have not addressed the issue for our users. CloudStack should have officially supported Java8 before Java7 reached EOL. True, true. I'm not against going to Java 8, I just don't want to abandon the Ubuntu users. The PPA however is a good way to go, that could go into the Release Notes and Upgrade docs. To minimize the impact of the change and smooth the transition, I suggest continuing to build with source and runtime compatibility set to 1.7 and test builds on both Java7 and Java8 until CloudStack 5. This approach will allow users to gradually transition from Java7 to Java8 with support for Java7 being eliminated at the next major release. Yes, that would be best. But don't we have a Java 8 Jenkins slave already? Wido Thanks, -John [1]: http://www.oracle.com/technetwork/java/eol-135779.html --- John Burwell (@john_burwell) VP of Software Engineering, ShapeBlue (571) 403-2411 | +44 20 3603 0542 http://www.shapeblue.com Please join us at CloudStack Collab EU — http://events.linuxfoundation.org/events/cloudstack-collaboration-conference-europe On Jul 15, 2015, at 7:57 AM, Wido den Hollander w...@widodh.nl wrote: Signed PGP part On 15-07-15 09:20, Wilder Rodrigues wrote: If there would be dependencies on some other things, that in no way could be fixed now, we could wait for 4.7 (5.0). However, if we could give it a go, I would be able to tackle this in our next Sprint (within 1 1/2 week from now) and still get it into 4.6. What would be the main considerations? Well, on Ubuntu systems we simply we will require a additional software repo. The good thing is that OpenJDK 8 is available for 12.04 and 14.04 I don't feel like we should do something like this in a 4.X release, but that's me. Wido Cheers, Wilder On 14 Jul 2015, at 22:25, Wido den Hollander w...@widodh.nlmailto:w...@widodh.nl wrote: On 07/14/2015 10:18 PM, John Burwell wrote: Wido, Is the OpenJDK PPA [1] not acceptable? Since Java7 is no longer supported, we run the risk of an Java security issue affecting the project that won’t be fixed. I didn't know that a PPA with OpenJDk existed. Looking at it I see that the maintainer Matthias Klose works for Canonical, so it seems like an official PPA. Still, having users adding PPAs is something we want to prevent as much as possible, but I do recognize the problem here. Ubuntu 16.04 is less then a year away and will have Java 8 support, so that will be resolved by then, but for now it is a problem. I think that CloudStack 4.6 is to early, but maybe 4.7 (called 5.0?) is a good time to make the move? Wido Thanks, -John [1]: https://launchpad.net/~openjdk-r/+archive/ubuntu/ppa --- John Burwell (@john_burwell) VP of Software Engineering, ShapeBlue (571) 403-2411 | +44 20 3603 0542 http://www.shapeblue.com Please join us at CloudStack Collab EU — http://events.linuxfoundation.org/events/cloudstack-collaboration-conf erence-europe On Jul 10, 2015, at 5:47 PM, Wido den Hollander w...@widodh.nlmailto:w...@widodh.nl wrote: Signed PGP part On 07/10/2015 09:22 PM, Rohit Yadav wrote: Ping Wilder - any progress/plan on moving forward (perhaps after 4.6?). I don't think there is? Since Ubuntu 14.04 doesn't support Java 8 in any package form we can't really continue. Ubuntu 16.04 will ship with Java 8 and that will be the next LTS. Wido On 01-May-2015, at 4:07 pm, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.comm ailto:wrodrig...@schubergphilis.com wrote: Hi Marcus, Sorry for the push, I think after doing the whole CitrixResourceBase refactor I also got a bit attached to the whole thing/solution. ;) Thanks for the input you gave. I will finish the refactor and apply it to both implementations. Cheers, Wilder On 01 May 2015, at 09:06, Marcus shadow...@gmail.commailto:shadow...@gmail.commailto:shadowsor@gmai l.commailto:shadowsor@gm ai l.comhttp://l.com wrote: Oh, and of course the annotation added to the wrapper looks like: ... @ResourceWrapper(handles = CheckHealthCommand.class) public final class LibvirtCheckHealthCommandWrapper ... maybe 'wraps' or 'wrapperfor' would be better than 'handles' in your naming scheme. You get the idea. On Thu, Apr 30, 2015 at 11:59 PM, Marcus shadow...@gmail.commailto:shadow...@gmail.commailto:shadowsor@gmai l.com wrote: I agree, this wrapper is a good step forward. It's totally fine to continue on that path because it is obviously better
Re: [DISCUSS] Moving to Java 8
Wido, We have an acute problem — Oracle will be issuing no further security patches for Java7 which is a significant opsec risk. Put simply, we can’t leave our users exposed to such a risk because Ubuntu won’t ship a release for an non-EOL’ed Java version for another year. Personally, I think it is a tremendous oversight that Java7 has been EOL’ed for over three (3) months [1], and we have not addressed the issue for our users. CloudStack should have officially supported Java8 before Java7 reached EOL. To minimize the impact of the change and smooth the transition, I suggest continuing to build with source and runtime compatibility set to 1.7 and test builds on both Java7 and Java8 until CloudStack 5. This approach will allow users to gradually transition from Java7 to Java8 with support for Java7 being eliminated at the next major release. Thanks, -John [1]: http://www.oracle.com/technetwork/java/eol-135779.html --- John Burwell (@john_burwell) VP of Software Engineering, ShapeBlue (571) 403-2411 | +44 20 3603 0542 http://www.shapeblue.com Please join us at CloudStack Collab EU — http://events.linuxfoundation.org/events/cloudstack-collaboration-conference-europe On Jul 15, 2015, at 7:57 AM, Wido den Hollander w...@widodh.nl wrote: Signed PGP part On 15-07-15 09:20, Wilder Rodrigues wrote: If there would be dependencies on some other things, that in no way could be fixed now, we could wait for 4.7 (5.0). However, if we could give it a go, I would be able to tackle this in our next Sprint (within 1 1/2 week from now) and still get it into 4.6. What would be the main considerations? Well, on Ubuntu systems we simply we will require a additional software repo. The good thing is that OpenJDK 8 is available for 12.04 and 14.04 I don't feel like we should do something like this in a 4.X release, but that's me. Wido Cheers, Wilder On 14 Jul 2015, at 22:25, Wido den Hollander w...@widodh.nlmailto:w...@widodh.nl wrote: On 07/14/2015 10:18 PM, John Burwell wrote: Wido, Is the OpenJDK PPA [1] not acceptable? Since Java7 is no longer supported, we run the risk of an Java security issue affecting the project that won’t be fixed. I didn't know that a PPA with OpenJDk existed. Looking at it I see that the maintainer Matthias Klose works for Canonical, so it seems like an official PPA. Still, having users adding PPAs is something we want to prevent as much as possible, but I do recognize the problem here. Ubuntu 16.04 is less then a year away and will have Java 8 support, so that will be resolved by then, but for now it is a problem. I think that CloudStack 4.6 is to early, but maybe 4.7 (called 5.0?) is a good time to make the move? Wido Thanks, -John [1]: https://launchpad.net/~openjdk-r/+archive/ubuntu/ppa --- John Burwell (@john_burwell) VP of Software Engineering, ShapeBlue (571) 403-2411 | +44 20 3603 0542 http://www.shapeblue.com Please join us at CloudStack Collab EU — http://events.linuxfoundation.org/events/cloudstack-collaboration-conf erence-europe On Jul 10, 2015, at 5:47 PM, Wido den Hollander w...@widodh.nlmailto:w...@widodh.nl wrote: Signed PGP part On 07/10/2015 09:22 PM, Rohit Yadav wrote: Ping Wilder - any progress/plan on moving forward (perhaps after 4.6?). I don't think there is? Since Ubuntu 14.04 doesn't support Java 8 in any package form we can't really continue. Ubuntu 16.04 will ship with Java 8 and that will be the next LTS. Wido On 01-May-2015, at 4:07 pm, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.comm ailto:wrodrig...@schubergphilis.com wrote: Hi Marcus, Sorry for the push, I think after doing the whole CitrixResourceBase refactor I also got a bit attached to the whole thing/solution. ;) Thanks for the input you gave. I will finish the refactor and apply it to both implementations. Cheers, Wilder On 01 May 2015, at 09:06, Marcus shadow...@gmail.commailto:shadow...@gmail.commailto:shadowsor@gmai l.commailto:shadowsor@gm ai l.comhttp://l.com wrote: Oh, and of course the annotation added to the wrapper looks like: ... @ResourceWrapper(handles = CheckHealthCommand.class) public final class LibvirtCheckHealthCommandWrapper ... maybe 'wraps' or 'wrapperfor' would be better than 'handles' in your naming scheme. You get the idea. On Thu, Apr 30, 2015 at 11:59 PM, Marcus shadow...@gmail.commailto:shadow...@gmail.commailto:shadowsor@gmai l.com wrote: I agree, this wrapper is a good step forward. It's totally fine to continue on that path because it is obviously better and makes it easy to switch to autodetection anytime later by simply adding the annotation. Sorry if I got a bit passionate about that, but as you mention I also get tired of adding things in multiple places, and the
Re: [DISCUSS] Moving to Java 8
If there would be dependencies on some other things, that in no way could be fixed now, we could wait for 4.7 (5.0). However, if we could give it a go, I would be able to tackle this in our next Sprint (within 1 1/2 week from now) and still get it into 4.6. What would be the main considerations? Cheers, Wilder On 14 Jul 2015, at 22:25, Wido den Hollander w...@widodh.nlmailto:w...@widodh.nl wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 07/14/2015 10:18 PM, John Burwell wrote: Wido, Is the OpenJDK PPA [1] not acceptable? Since Java7 is no longer supported, we run the risk of an Java security issue affecting the project that won’t be fixed. I didn't know that a PPA with OpenJDk existed. Looking at it I see that the maintainer Matthias Klose works for Canonical, so it seems like an official PPA. Still, having users adding PPAs is something we want to prevent as much as possible, but I do recognize the problem here. Ubuntu 16.04 is less then a year away and will have Java 8 support, so that will be resolved by then, but for now it is a problem. I think that CloudStack 4.6 is to early, but maybe 4.7 (called 5.0?) is a good time to make the move? Wido Thanks, -John [1]: https://launchpad.net/~openjdk-r/+archive/ubuntu/ppa --- John Burwell (@john_burwell) VP of Software Engineering, ShapeBlue (571) 403-2411 | +44 20 3603 0542 http://www.shapeblue.com Please join us at CloudStack Collab EU — http://events.linuxfoundation.org/events/cloudstack-collaboration-conf erence-europe On Jul 10, 2015, at 5:47 PM, Wido den Hollander w...@widodh.nlmailto:w...@widodh.nl wrote: Signed PGP part On 07/10/2015 09:22 PM, Rohit Yadav wrote: Ping Wilder - any progress/plan on moving forward (perhaps after 4.6?). I don't think there is? Since Ubuntu 14.04 doesn't support Java 8 in any package form we can't really continue. Ubuntu 16.04 will ship with Java 8 and that will be the next LTS. Wido On 01-May-2015, at 4:07 pm, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.com wrote: Hi Marcus, Sorry for the push, I think after doing the whole CitrixResourceBase refactor I also got a bit attached to the whole thing/solution. ;) Thanks for the input you gave. I will finish the refactor and apply it to both implementations. Cheers, Wilder On 01 May 2015, at 09:06, Marcus shadow...@gmail.commailto:shadow...@gmail.commailto:shadow...@gmail.commailto:shadowsor@gm ai l.comhttp://l.com wrote: Oh, and of course the annotation added to the wrapper looks like: ... @ResourceWrapper(handles = CheckHealthCommand.class) public final class LibvirtCheckHealthCommandWrapper ... maybe 'wraps' or 'wrapperfor' would be better than 'handles' in your naming scheme. You get the idea. On Thu, Apr 30, 2015 at 11:59 PM, Marcus shadow...@gmail.commailto:shadow...@gmail.commailto:shadow...@gmail.com wrote: I agree, this wrapper is a good step forward. It's totally fine to continue on that path because it is obviously better and makes it easy to switch to autodetection anytime later by simply adding the annotation. Sorry if I got a bit passionate about that, but as you mention I also get tired of adding things in multiple places, and the annotations have worked well in the API and provide a good model to emulate for consistency. I can't share code, because these extensions to LibvirtComputingResource that I've provided for other companies have not been open sourced. I can speak more generically though about methods. To answer question a, reflection allows you to do something like: Reflections reflections = new Reflections(com.cloud.hypervisor.kvm.resource.wrapper); SetClass? extends CommandWrapper wrappers = reflections.getSubTypesOf(CommandWrapper.class); So here in new Reflections we are automatically filtering for just the wrappers that would apply to the KVM plugin. Then to finish it off, you iterate through the wrappers and do: ResourceWrapper annotation = wrapper.getAnnotation(ResourceWrapper.class); citrixCommands.put(annotation.handles(), wrapper.newInstance()); Sorry, I guess that's four lines, plus the relevant for loop. And probably a null check or something for the annotation. You also have to add the annotation class itself, and add a line for the annotation in each wrapper, but in the end when we add new Commands, we won't have to touch anything but the new class that handles the command. public @interface ResourceWrapper { Class? extends Command handles(); } There's an example of something similar to this in KVMStoragePoolManager.java (annotation is StoragePoolInfo.java). This example has actually been adapted from that. Also to a lesser extent in the API server, but it is spread across a bunch of classes. On Thu, Apr 30, 2015 at 10:41 PM, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.com wrote: Hi Marcus, Thanks for the email… I’m
Re: [DISCUSS] Moving to Java 8
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 15-07-15 09:20, Wilder Rodrigues wrote: If there would be dependencies on some other things, that in no way could be fixed now, we could wait for 4.7 (5.0). However, if we could give it a go, I would be able to tackle this in our next Sprint (within 1 1/2 week from now) and still get it into 4.6. What would be the main considerations? Well, on Ubuntu systems we simply we will require a additional software repo. The good thing is that OpenJDK 8 is available for 12.04 and 14.04 I don't feel like we should do something like this in a 4.X release, but that's me. Wido Cheers, Wilder On 14 Jul 2015, at 22:25, Wido den Hollander w...@widodh.nlmailto:w...@widodh.nl wrote: On 07/14/2015 10:18 PM, John Burwell wrote: Wido, Is the OpenJDK PPA [1] not acceptable? Since Java7 is no longer supported, we run the risk of an Java security issue affecting the project that won’t be fixed. I didn't know that a PPA with OpenJDk existed. Looking at it I see that the maintainer Matthias Klose works for Canonical, so it seems like an official PPA. Still, having users adding PPAs is something we want to prevent as much as possible, but I do recognize the problem here. Ubuntu 16.04 is less then a year away and will have Java 8 support, so that will be resolved by then, but for now it is a problem. I think that CloudStack 4.6 is to early, but maybe 4.7 (called 5.0?) is a good time to make the move? Wido Thanks, -John [1]: https://launchpad.net/~openjdk-r/+archive/ubuntu/ppa --- John Burwell (@john_burwell) VP of Software Engineering, ShapeBlue (571) 403-2411 | +44 20 3603 0542 http://www.shapeblue.com Please join us at CloudStack Collab EU — http://events.linuxfoundation.org/events/cloudstack-collaboration-conf erence-europe On Jul 10, 2015, at 5:47 PM, Wido den Hollander w...@widodh.nlmailto:w...@widodh.nl wrote: Signed PGP part On 07/10/2015 09:22 PM, Rohit Yadav wrote: Ping Wilder - any progress/plan on moving forward (perhaps after 4.6?). I don't think there is? Since Ubuntu 14.04 doesn't support Java 8 in any package form we can't really continue. Ubuntu 16.04 will ship with Java 8 and that will be the next LTS. Wido On 01-May-2015, at 4:07 pm, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.comm ailto:wrodrig...@schubergphilis.com wrote: Hi Marcus, Sorry for the push, I think after doing the whole CitrixResourceBase refactor I also got a bit attached to the whole thing/solution. ;) Thanks for the input you gave. I will finish the refactor and apply it to both implementations. Cheers, Wilder On 01 May 2015, at 09:06, Marcus shadow...@gmail.commailto:shadow...@gmail.commailto:shadowsor@gmai l.commailto:shadowsor@gm ai l.comhttp://l.com wrote: Oh, and of course the annotation added to the wrapper looks like: ... @ResourceWrapper(handles = CheckHealthCommand.class) public final class LibvirtCheckHealthCommandWrapper ... maybe 'wraps' or 'wrapperfor' would be better than 'handles' in your naming scheme. You get the idea. On Thu, Apr 30, 2015 at 11:59 PM, Marcus shadow...@gmail.commailto:shadow...@gmail.commailto:shadowsor@gmai l.com wrote: I agree, this wrapper is a good step forward. It's totally fine to continue on that path because it is obviously better and makes it easy to switch to autodetection anytime later by simply adding the annotation. Sorry if I got a bit passionate about that, but as you mention I also get tired of adding things in multiple places, and the annotations have worked well in the API and provide a good model to emulate for consistency. I can't share code, because these extensions to LibvirtComputingResource that I've provided for other companies have not been open sourced. I can speak more generically though about methods. To answer question a, reflection allows you to do something like: Reflections reflections = new Reflections(com.cloud.hypervisor.kvm.resource.wrapper); SetClass? extends CommandWrapper wrappers = reflections.getSubTypesOf(CommandWrapper.class); So here in new Reflections we are automatically filtering for just the wrappers that would apply to the KVM plugin. Then to finish it off, you iterate through the wrappers and do: ResourceWrapper annotation = wrapper.getAnnotation(ResourceWrapper.class); citrixCommands.put(annotation.handles(), wrapper.newInstance()); Sorry, I guess that's four lines, plus the relevant for loop. And probably a null check or something for the annotation. You also have to add the annotation class itself, and add a line for the annotation in each wrapper, but in the end when we add new Commands, we won't have to touch anything but the new class that handles the command. public @interface ResourceWrapper { Class? extends Command handles(); }
Re: [DISCUSS] Moving to Java 8
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 07/14/2015 10:18 PM, John Burwell wrote: Wido, Is the OpenJDK PPA [1] not acceptable? Since Java7 is no longer supported, we run the risk of an Java security issue affecting the project that won’t be fixed. I didn't know that a PPA with OpenJDk existed. Looking at it I see that the maintainer Matthias Klose works for Canonical, so it seems like an official PPA. Still, having users adding PPAs is something we want to prevent as much as possible, but I do recognize the problem here. Ubuntu 16.04 is less then a year away and will have Java 8 support, so that will be resolved by then, but for now it is a problem. I think that CloudStack 4.6 is to early, but maybe 4.7 (called 5.0?) is a good time to make the move? Wido Thanks, -John [1]: https://launchpad.net/~openjdk-r/+archive/ubuntu/ppa --- John Burwell (@john_burwell) VP of Software Engineering, ShapeBlue (571) 403-2411 | +44 20 3603 0542 http://www.shapeblue.com Please join us at CloudStack Collab EU — http://events.linuxfoundation.org/events/cloudstack-collaboration-conf erence-europe On Jul 10, 2015, at 5:47 PM, Wido den Hollander w...@widodh.nl wrote: Signed PGP part On 07/10/2015 09:22 PM, Rohit Yadav wrote: Ping Wilder - any progress/plan on moving forward (perhaps after 4.6?). I don't think there is? Since Ubuntu 14.04 doesn't support Java 8 in any package form we can't really continue. Ubuntu 16.04 will ship with Java 8 and that will be the next LTS. Wido On 01-May-2015, at 4:07 pm, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.com wrote: Hi Marcus, Sorry for the push, I think after doing the whole CitrixResourceBase refactor I also got a bit attached to the whole thing/solution. ;) Thanks for the input you gave. I will finish the refactor and apply it to both implementations. Cheers, Wilder On 01 May 2015, at 09:06, Marcus shadow...@gmail.commailto:shadow...@gmail.commailto:shadowsor@gm ai l.com wrote: Oh, and of course the annotation added to the wrapper looks like: ... @ResourceWrapper(handles = CheckHealthCommand.class) public final class LibvirtCheckHealthCommandWrapper ... maybe 'wraps' or 'wrapperfor' would be better than 'handles' in your naming scheme. You get the idea. On Thu, Apr 30, 2015 at 11:59 PM, Marcus shadow...@gmail.commailto:shadow...@gmail.com wrote: I agree, this wrapper is a good step forward. It's totally fine to continue on that path because it is obviously better and makes it easy to switch to autodetection anytime later by simply adding the annotation. Sorry if I got a bit passionate about that, but as you mention I also get tired of adding things in multiple places, and the annotations have worked well in the API and provide a good model to emulate for consistency. I can't share code, because these extensions to LibvirtComputingResource that I've provided for other companies have not been open sourced. I can speak more generically though about methods. To answer question a, reflection allows you to do something like: Reflections reflections = new Reflections(com.cloud.hypervisor.kvm.resource.wrapper); SetClass? extends CommandWrapper wrappers = reflections.getSubTypesOf(CommandWrapper.class); So here in new Reflections we are automatically filtering for just the wrappers that would apply to the KVM plugin. Then to finish it off, you iterate through the wrappers and do: ResourceWrapper annotation = wrapper.getAnnotation(ResourceWrapper.class); citrixCommands.put(annotation.handles(), wrapper.newInstance()); Sorry, I guess that's four lines, plus the relevant for loop. And probably a null check or something for the annotation. You also have to add the annotation class itself, and add a line for the annotation in each wrapper, but in the end when we add new Commands, we won't have to touch anything but the new class that handles the command. public @interface ResourceWrapper { Class? extends Command handles(); } There's an example of something similar to this in KVMStoragePoolManager.java (annotation is StoragePoolInfo.java). This example has actually been adapted from that. Also to a lesser extent in the API server, but it is spread across a bunch of classes. On Thu, Apr 30, 2015 at 10:41 PM, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.com wrote: Hi Marcus, Thanks for the email… I’m always in for improvements. But why can’t you share the code? Few points below: 1. I added an subclassing example of LibvirtComputingResource because you mentioned it in a previous email: On 23 Apr 2015, at 17:26, Marcus shadow...@gmail.commailto:shadow...@gmail.com wrote: I mentioned the reflection model because that's how I tend to handle the commands when subclassing LibvirtComputingResource. 2.
Re: [DISCUSS] Moving to Java 8
Wido, Is the OpenJDK PPA [1] not acceptable? Since Java7 is no longer supported, we run the risk of an Java security issue affecting the project that won’t be fixed. Thanks, -John [1]: https://launchpad.net/~openjdk-r/+archive/ubuntu/ppa --- John Burwell (@john_burwell) VP of Software Engineering, ShapeBlue (571) 403-2411 | +44 20 3603 0542 http://www.shapeblue.com Please join us at CloudStack Collab EU — http://events.linuxfoundation.org/events/cloudstack-collaboration-conference-europe On Jul 10, 2015, at 5:47 PM, Wido den Hollander w...@widodh.nl wrote: Signed PGP part On 07/10/2015 09:22 PM, Rohit Yadav wrote: Ping Wilder - any progress/plan on moving forward (perhaps after 4.6?). I don't think there is? Since Ubuntu 14.04 doesn't support Java 8 in any package form we can't really continue. Ubuntu 16.04 will ship with Java 8 and that will be the next LTS. Wido On 01-May-2015, at 4:07 pm, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.com wrote: Hi Marcus, Sorry for the push, I think after doing the whole CitrixResourceBase refactor I also got a bit attached to the whole thing/solution. ;) Thanks for the input you gave. I will finish the refactor and apply it to both implementations. Cheers, Wilder On 01 May 2015, at 09:06, Marcus shadow...@gmail.commailto:shadow...@gmail.commailto:shadowsor@gmai l.com wrote: Oh, and of course the annotation added to the wrapper looks like: ... @ResourceWrapper(handles = CheckHealthCommand.class) public final class LibvirtCheckHealthCommandWrapper ... maybe 'wraps' or 'wrapperfor' would be better than 'handles' in your naming scheme. You get the idea. On Thu, Apr 30, 2015 at 11:59 PM, Marcus shadow...@gmail.commailto:shadow...@gmail.com wrote: I agree, this wrapper is a good step forward. It's totally fine to continue on that path because it is obviously better and makes it easy to switch to autodetection anytime later by simply adding the annotation. Sorry if I got a bit passionate about that, but as you mention I also get tired of adding things in multiple places, and the annotations have worked well in the API and provide a good model to emulate for consistency. I can't share code, because these extensions to LibvirtComputingResource that I've provided for other companies have not been open sourced. I can speak more generically though about methods. To answer question a, reflection allows you to do something like: Reflections reflections = new Reflections(com.cloud.hypervisor.kvm.resource.wrapper); SetClass? extends CommandWrapper wrappers = reflections.getSubTypesOf(CommandWrapper.class); So here in new Reflections we are automatically filtering for just the wrappers that would apply to the KVM plugin. Then to finish it off, you iterate through the wrappers and do: ResourceWrapper annotation = wrapper.getAnnotation(ResourceWrapper.class); citrixCommands.put(annotation.handles(), wrapper.newInstance()); Sorry, I guess that's four lines, plus the relevant for loop. And probably a null check or something for the annotation. You also have to add the annotation class itself, and add a line for the annotation in each wrapper, but in the end when we add new Commands, we won't have to touch anything but the new class that handles the command. public @interface ResourceWrapper { Class? extends Command handles(); } There's an example of something similar to this in KVMStoragePoolManager.java (annotation is StoragePoolInfo.java). This example has actually been adapted from that. Also to a lesser extent in the API server, but it is spread across a bunch of classes. On Thu, Apr 30, 2015 at 10:41 PM, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.com wrote: Hi Marcus, Thanks for the email… I’m always in for improvements. But why can’t you share the code? Few points below: 1. I added an subclassing example of LibvirtComputingResource because you mentioned it in a previous email: On 23 Apr 2015, at 17:26, Marcus shadow...@gmail.commailto:shadow...@gmail.com wrote: I mentioned the reflection model because that's how I tend to handle the commands when subclassing LibvirtComputingResource. 2. Current situation with LibvirtComputingResource on Master is: a. 67 IFs b. 67 private/protected methods that are used only there c. If a new Command is added it means we will have a new IF and a new private method e. Maintenance is hell, test is close to zero and code quality is below expectations That being said, the main idea with the refactor is to change structure only, not behaviour. So what I’m doing is to simply move the code out the LibvirtCompRes and write tests for it, keeping the behaviour the same - to be done in a next phase. If you look at the changes you
Re: [DISCUSS] Moving to Java 8
Ping Wilder - any progress/plan on moving forward (perhaps after 4.6?). On 01-May-2015, at 4:07 pm, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.com wrote: Hi Marcus, Sorry for the push, I think after doing the whole CitrixResourceBase refactor I also got a bit attached to the whole thing/solution. ;) Thanks for the input you gave. I will finish the refactor and apply it to both implementations. Cheers, Wilder On 01 May 2015, at 09:06, Marcus shadow...@gmail.commailto:shadow...@gmail.commailto:shadow...@gmail.com wrote: Oh, and of course the annotation added to the wrapper looks like: ... @ResourceWrapper(handles = CheckHealthCommand.class) public final class LibvirtCheckHealthCommandWrapper ... maybe 'wraps' or 'wrapperfor' would be better than 'handles' in your naming scheme. You get the idea. On Thu, Apr 30, 2015 at 11:59 PM, Marcus shadow...@gmail.commailto:shadow...@gmail.com wrote: I agree, this wrapper is a good step forward. It's totally fine to continue on that path because it is obviously better and makes it easy to switch to autodetection anytime later by simply adding the annotation. Sorry if I got a bit passionate about that, but as you mention I also get tired of adding things in multiple places, and the annotations have worked well in the API and provide a good model to emulate for consistency. I can't share code, because these extensions to LibvirtComputingResource that I've provided for other companies have not been open sourced. I can speak more generically though about methods. To answer question a, reflection allows you to do something like: Reflections reflections = new Reflections(com.cloud.hypervisor.kvm.resource.wrapper); SetClass? extends CommandWrapper wrappers = reflections.getSubTypesOf(CommandWrapper.class); So here in new Reflections we are automatically filtering for just the wrappers that would apply to the KVM plugin. Then to finish it off, you iterate through the wrappers and do: ResourceWrapper annotation = wrapper.getAnnotation(ResourceWrapper.class); citrixCommands.put(annotation.handles(), wrapper.newInstance()); Sorry, I guess that's four lines, plus the relevant for loop. And probably a null check or something for the annotation. You also have to add the annotation class itself, and add a line for the annotation in each wrapper, but in the end when we add new Commands, we won't have to touch anything but the new class that handles the command. public @interface ResourceWrapper { Class? extends Command handles(); } There's an example of something similar to this in KVMStoragePoolManager.java (annotation is StoragePoolInfo.java). This example has actually been adapted from that. Also to a lesser extent in the API server, but it is spread across a bunch of classes. On Thu, Apr 30, 2015 at 10:41 PM, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.com wrote: Hi Marcus, Thanks for the email… I’m always in for improvements. But why can’t you share the code? Few points below: 1. I added an subclassing example of LibvirtComputingResource because you mentioned it in a previous email: On 23 Apr 2015, at 17:26, Marcus shadow...@gmail.commailto:shadow...@gmail.com wrote: I mentioned the reflection model because that's how I tend to handle the commands when subclassing LibvirtComputingResource. 2. Current situation with LibvirtComputingResource on Master is: a. 67 IFs b. 67 private/protected methods that are used only there c. If a new Command is added it means we will have a new IF and a new private method e. Maintenance is hell, test is close to zero and code quality is below expectations That being said, the main idea with the refactor is to change structure only, not behaviour. So what I’m doing is to simply move the code out the LibvirtCompRes and write tests for it, keeping the behaviour the same - to be done in a next phase. If you look at the changes you will see that some wrappers are already 100% covered. However, some others have 4% or 8% (not that much though). I would like to refactor that as well, but that could change behaviour (mentioned above) which I don’t want to touch now. 3. With the new situation: a. No IFs b. All methods wrapped by other classes (command wrappers) - loosely coupled, easier to test and maintain c. If a new Command is added we would have to add a command wrapper and 1 line in the request wrapper implementation ( I know, it hurts you a bit) - but please bear with me for the good news. 4. the warnings are due to that: HashtableClass? extends Command, CommandWrapper() No big deal. As I understood from your first paragraph we would have to annotated the commands classes, right? I mean, all of them. That’s something I wouldn’t do in this phase, to be honest. It might seem harmless to do, but I like to break things down a bit and have more isolation in my changes. What’s next: I will finish
Re: [DISCUSS] Moving to Java 8
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 07/10/2015 09:22 PM, Rohit Yadav wrote: Ping Wilder - any progress/plan on moving forward (perhaps after 4.6?). I don't think there is? Since Ubuntu 14.04 doesn't support Java 8 in any package form we can't really continue. Ubuntu 16.04 will ship with Java 8 and that will be the next LTS. Wido On 01-May-2015, at 4:07 pm, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.com wrote: Hi Marcus, Sorry for the push, I think after doing the whole CitrixResourceBase refactor I also got a bit attached to the whole thing/solution. ;) Thanks for the input you gave. I will finish the refactor and apply it to both implementations. Cheers, Wilder On 01 May 2015, at 09:06, Marcus shadow...@gmail.commailto:shadow...@gmail.commailto:shadowsor@gmai l.com wrote: Oh, and of course the annotation added to the wrapper looks like: ... @ResourceWrapper(handles = CheckHealthCommand.class) public final class LibvirtCheckHealthCommandWrapper ... maybe 'wraps' or 'wrapperfor' would be better than 'handles' in your naming scheme. You get the idea. On Thu, Apr 30, 2015 at 11:59 PM, Marcus shadow...@gmail.commailto:shadow...@gmail.com wrote: I agree, this wrapper is a good step forward. It's totally fine to continue on that path because it is obviously better and makes it easy to switch to autodetection anytime later by simply adding the annotation. Sorry if I got a bit passionate about that, but as you mention I also get tired of adding things in multiple places, and the annotations have worked well in the API and provide a good model to emulate for consistency. I can't share code, because these extensions to LibvirtComputingResource that I've provided for other companies have not been open sourced. I can speak more generically though about methods. To answer question a, reflection allows you to do something like: Reflections reflections = new Reflections(com.cloud.hypervisor.kvm.resource.wrapper); SetClass? extends CommandWrapper wrappers = reflections.getSubTypesOf(CommandWrapper.class); So here in new Reflections we are automatically filtering for just the wrappers that would apply to the KVM plugin. Then to finish it off, you iterate through the wrappers and do: ResourceWrapper annotation = wrapper.getAnnotation(ResourceWrapper.class); citrixCommands.put(annotation.handles(), wrapper.newInstance()); Sorry, I guess that's four lines, plus the relevant for loop. And probably a null check or something for the annotation. You also have to add the annotation class itself, and add a line for the annotation in each wrapper, but in the end when we add new Commands, we won't have to touch anything but the new class that handles the command. public @interface ResourceWrapper { Class? extends Command handles(); } There's an example of something similar to this in KVMStoragePoolManager.java (annotation is StoragePoolInfo.java). This example has actually been adapted from that. Also to a lesser extent in the API server, but it is spread across a bunch of classes. On Thu, Apr 30, 2015 at 10:41 PM, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.com wrote: Hi Marcus, Thanks for the email… I’m always in for improvements. But why can’t you share the code? Few points below: 1. I added an subclassing example of LibvirtComputingResource because you mentioned it in a previous email: On 23 Apr 2015, at 17:26, Marcus shadow...@gmail.commailto:shadow...@gmail.com wrote: I mentioned the reflection model because that's how I tend to handle the commands when subclassing LibvirtComputingResource. 2. Current situation with LibvirtComputingResource on Master is: a. 67 IFs b. 67 private/protected methods that are used only there c. If a new Command is added it means we will have a new IF and a new private method e. Maintenance is hell, test is close to zero and code quality is below expectations That being said, the main idea with the refactor is to change structure only, not behaviour. So what I’m doing is to simply move the code out the LibvirtCompRes and write tests for it, keeping the behaviour the same - to be done in a next phase. If you look at the changes you will see that some wrappers are already 100% covered. However, some others have 4% or 8% (not that much though). I would like to refactor that as well, but that could change behaviour (mentioned above) which I don’t want to touch now. 3. With the new situation: a. No IFs b. All methods wrapped by other classes (command wrappers) - loosely coupled, easier to test and maintain c. If a new Command is added we would have to add a command wrapper and 1 line in the request wrapper implementation ( I know, it hurts you a bit) - but please bear with me for the good news. 4. the warnings are due to that: HashtableClass?
Re: [DISCUSS] Moving to Java 8
Oh, and of course the annotation added to the wrapper looks like: ... @ResourceWrapper(handles = CheckHealthCommand.class) public final class LibvirtCheckHealthCommandWrapper ... maybe 'wraps' or 'wrapperfor' would be better than 'handles' in your naming scheme. You get the idea. On Thu, Apr 30, 2015 at 11:59 PM, Marcus shadow...@gmail.com wrote: I agree, this wrapper is a good step forward. It's totally fine to continue on that path because it is obviously better and makes it easy to switch to autodetection anytime later by simply adding the annotation. Sorry if I got a bit passionate about that, but as you mention I also get tired of adding things in multiple places, and the annotations have worked well in the API and provide a good model to emulate for consistency. I can't share code, because these extensions to LibvirtComputingResource that I've provided for other companies have not been open sourced. I can speak more generically though about methods. To answer question a, reflection allows you to do something like: Reflections reflections = new Reflections(com.cloud.hypervisor.kvm.resource.wrapper); SetClass? extends CommandWrapper wrappers = reflections.getSubTypesOf(CommandWrapper.class); So here in new Reflections we are automatically filtering for just the wrappers that would apply to the KVM plugin. Then to finish it off, you iterate through the wrappers and do: ResourceWrapper annotation = wrapper.getAnnotation(ResourceWrapper.class); citrixCommands.put(annotation.handles(), wrapper.newInstance()); Sorry, I guess that's four lines, plus the relevant for loop. And probably a null check or something for the annotation. You also have to add the annotation class itself, and add a line for the annotation in each wrapper, but in the end when we add new Commands, we won't have to touch anything but the new class that handles the command. public @interface ResourceWrapper { Class? extends Command handles(); } There's an example of something similar to this in KVMStoragePoolManager.java (annotation is StoragePoolInfo.java). This example has actually been adapted from that. Also to a lesser extent in the API server, but it is spread across a bunch of classes. On Thu, Apr 30, 2015 at 10:41 PM, Wilder Rodrigues wrodrig...@schubergphilis.com wrote: Hi Marcus, Thanks for the email… I’m always in for improvements. But why can’t you share the code? Few points below: 1. I added an subclassing example of LibvirtComputingResource because you mentioned it in a previous email: On 23 Apr 2015, at 17:26, Marcus shadow...@gmail.com wrote: I mentioned the reflection model because that's how I tend to handle the commands when subclassing LibvirtComputingResource. 2. Current situation with LibvirtComputingResource on Master is: a. 67 IFs b. 67 private/protected methods that are used only there c. If a new Command is added it means we will have a new IF and a new private method e. Maintenance is hell, test is close to zero and code quality is below expectations That being said, the main idea with the refactor is to change structure only, not behaviour. So what I’m doing is to simply move the code out the LibvirtCompRes and write tests for it, keeping the behaviour the same - to be done in a next phase. If you look at the changes you will see that some wrappers are already 100% covered. However, some others have 4% or 8% (not that much though). I would like to refactor that as well, but that could change behaviour (mentioned above) which I don’t want to touch now. 3. With the new situation: a. No IFs b. All methods wrapped by other classes (command wrappers) - loosely coupled, easier to test and maintain c. If a new Command is added we would have to add a command wrapper and 1 line in the request wrapper implementation ( I know, it hurts you a bit) - but please bear with me for the good news. 4. the warnings are due to that: HashtableClass? extends Command, CommandWrapper() No big deal. As I understood from your first paragraph we would have to annotated the commands classes, right? I mean, all of them. That’s something I wouldn’t do in this phase, to be honest. It might seem harmless to do, but I like to break things down a bit and have more isolation in my changes. What’s next: I will finish the refactor with the request wrapper as it is. For me it is no problem do add the lines now and remove them in 1 week. Most of the work is concentrated in the tests, which I’m trying as hard as I can to get them in the best way possible. Once it’s done and pushed to master, I will analyse what we would need to apply the annotation. But before I go to bring the kids to school, just one question: a. The “handle” value, in the annotation, would have the wrapper class that would be used for that command, right? Now let’s get 1 command as example: CheckHealthCommand. Its wrapper
Re: [DISCUSS] Moving to Java 8
I agree, this wrapper is a good step forward. It's totally fine to continue on that path because it is obviously better and makes it easy to switch to autodetection anytime later by simply adding the annotation. Sorry if I got a bit passionate about that, but as you mention I also get tired of adding things in multiple places, and the annotations have worked well in the API and provide a good model to emulate for consistency. I can't share code, because these extensions to LibvirtComputingResource that I've provided for other companies have not been open sourced. I can speak more generically though about methods. To answer question a, reflection allows you to do something like: Reflections reflections = new Reflections(com.cloud.hypervisor.kvm.resource.wrapper); SetClass? extends CommandWrapper wrappers = reflections.getSubTypesOf(CommandWrapper.class); So here in new Reflections we are automatically filtering for just the wrappers that would apply to the KVM plugin. Then to finish it off, you iterate through the wrappers and do: ResourceWrapper annotation = wrapper.getAnnotation(ResourceWrapper.class); citrixCommands.put(annotation.handles(), wrapper.newInstance()); Sorry, I guess that's four lines, plus the relevant for loop. And probably a null check or something for the annotation. You also have to add the annotation class itself, and add a line for the annotation in each wrapper, but in the end when we add new Commands, we won't have to touch anything but the new class that handles the command. public @interface ResourceWrapper { Class? extends Command handles(); } There's an example of something similar to this in KVMStoragePoolManager.java (annotation is StoragePoolInfo.java). This example has actually been adapted from that. Also to a lesser extent in the API server, but it is spread across a bunch of classes. On Thu, Apr 30, 2015 at 10:41 PM, Wilder Rodrigues wrodrig...@schubergphilis.com wrote: Hi Marcus, Thanks for the email… I’m always in for improvements. But why can’t you share the code? Few points below: 1. I added an subclassing example of LibvirtComputingResource because you mentioned it in a previous email: On 23 Apr 2015, at 17:26, Marcus shadow...@gmail.com wrote: I mentioned the reflection model because that's how I tend to handle the commands when subclassing LibvirtComputingResource. 2. Current situation with LibvirtComputingResource on Master is: a. 67 IFs b. 67 private/protected methods that are used only there c. If a new Command is added it means we will have a new IF and a new private method e. Maintenance is hell, test is close to zero and code quality is below expectations That being said, the main idea with the refactor is to change structure only, not behaviour. So what I’m doing is to simply move the code out the LibvirtCompRes and write tests for it, keeping the behaviour the same - to be done in a next phase. If you look at the changes you will see that some wrappers are already 100% covered. However, some others have 4% or 8% (not that much though). I would like to refactor that as well, but that could change behaviour (mentioned above) which I don’t want to touch now. 3. With the new situation: a. No IFs b. All methods wrapped by other classes (command wrappers) - loosely coupled, easier to test and maintain c. If a new Command is added we would have to add a command wrapper and 1 line in the request wrapper implementation ( I know, it hurts you a bit) - but please bear with me for the good news. 4. the warnings are due to that: HashtableClass? extends Command, CommandWrapper() No big deal. As I understood from your first paragraph we would have to annotated the commands classes, right? I mean, all of them. That’s something I wouldn’t do in this phase, to be honest. It might seem harmless to do, but I like to break things down a bit and have more isolation in my changes. What’s next: I will finish the refactor with the request wrapper as it is. For me it is no problem do add the lines now and remove them in 1 week. Most of the work is concentrated in the tests, which I’m trying as hard as I can to get them in the best way possible. Once it’s done and pushed to master, I will analyse what we would need to apply the annotation. But before I go to bring the kids to school, just one question: a. The “handle” value, in the annotation, would have the wrapper class that would be used for that command, right? Now let’s get 1 command as example: CheckHealthCommand. Its wrapper implementation differs per hypervisor (just like all the other wrapper commands do). I’m not taking the time to really think about it now, but how would we annotated the different wrappers per command? Thanks again for your time. Cheers, Wilder On 30 Apr 2015, at 22:52, Marcus shadow...@gmail.com wrote: Ok. I wish I could share some code, because it isn't really as big of a
Re: [DISCUSS] Moving to Java 8
Hi Marcus, Sorry for the push, I think after doing the whole CitrixResourceBase refactor I also got a bit attached to the whole thing/solution. ;) Thanks for the input you gave. I will finish the refactor and apply it to both implementations. Cheers, Wilder On 01 May 2015, at 09:06, Marcus shadow...@gmail.commailto:shadow...@gmail.com wrote: Oh, and of course the annotation added to the wrapper looks like: ... @ResourceWrapper(handles = CheckHealthCommand.class) public final class LibvirtCheckHealthCommandWrapper ... maybe 'wraps' or 'wrapperfor' would be better than 'handles' in your naming scheme. You get the idea. On Thu, Apr 30, 2015 at 11:59 PM, Marcus shadow...@gmail.commailto:shadow...@gmail.com wrote: I agree, this wrapper is a good step forward. It's totally fine to continue on that path because it is obviously better and makes it easy to switch to autodetection anytime later by simply adding the annotation. Sorry if I got a bit passionate about that, but as you mention I also get tired of adding things in multiple places, and the annotations have worked well in the API and provide a good model to emulate for consistency. I can't share code, because these extensions to LibvirtComputingResource that I've provided for other companies have not been open sourced. I can speak more generically though about methods. To answer question a, reflection allows you to do something like: Reflections reflections = new Reflections(com.cloud.hypervisor.kvm.resource.wrapper); SetClass? extends CommandWrapper wrappers = reflections.getSubTypesOf(CommandWrapper.class); So here in new Reflections we are automatically filtering for just the wrappers that would apply to the KVM plugin. Then to finish it off, you iterate through the wrappers and do: ResourceWrapper annotation = wrapper.getAnnotation(ResourceWrapper.class); citrixCommands.put(annotation.handles(), wrapper.newInstance()); Sorry, I guess that's four lines, plus the relevant for loop. And probably a null check or something for the annotation. You also have to add the annotation class itself, and add a line for the annotation in each wrapper, but in the end when we add new Commands, we won't have to touch anything but the new class that handles the command. public @interface ResourceWrapper { Class? extends Command handles(); } There's an example of something similar to this in KVMStoragePoolManager.java (annotation is StoragePoolInfo.java). This example has actually been adapted from that. Also to a lesser extent in the API server, but it is spread across a bunch of classes. On Thu, Apr 30, 2015 at 10:41 PM, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.com wrote: Hi Marcus, Thanks for the email… I’m always in for improvements. But why can’t you share the code? Few points below: 1. I added an subclassing example of LibvirtComputingResource because you mentioned it in a previous email: On 23 Apr 2015, at 17:26, Marcus shadow...@gmail.commailto:shadow...@gmail.com wrote: I mentioned the reflection model because that's how I tend to handle the commands when subclassing LibvirtComputingResource. 2. Current situation with LibvirtComputingResource on Master is: a. 67 IFs b. 67 private/protected methods that are used only there c. If a new Command is added it means we will have a new IF and a new private method e. Maintenance is hell, test is close to zero and code quality is below expectations That being said, the main idea with the refactor is to change structure only, not behaviour. So what I’m doing is to simply move the code out the LibvirtCompRes and write tests for it, keeping the behaviour the same - to be done in a next phase. If you look at the changes you will see that some wrappers are already 100% covered. However, some others have 4% or 8% (not that much though). I would like to refactor that as well, but that could change behaviour (mentioned above) which I don’t want to touch now. 3. With the new situation: a. No IFs b. All methods wrapped by other classes (command wrappers) - loosely coupled, easier to test and maintain c. If a new Command is added we would have to add a command wrapper and 1 line in the request wrapper implementation ( I know, it hurts you a bit) - but please bear with me for the good news. 4. the warnings are due to that: HashtableClass? extends Command, CommandWrapper() No big deal. As I understood from your first paragraph we would have to annotated the commands classes, right? I mean, all of them. That’s something I wouldn’t do in this phase, to be honest. It might seem harmless to do, but I like to break things down a bit and have more isolation in my changes. What’s next: I will finish the refactor with the request wrapper as it is. For me it is no problem do add the lines now and remove them in 1 week. Most of the work is concentrated in the tests, which I’m trying as hard as I can to get them
Re: [DISCUSS] Moving to Java 8
Hi Marcus, Apologies for taking so much time to reply to your email, but was, and still am, quite busy. :) I would only use reflection if that was the only way to do it. The use of reflection usually makes the code more complex, which is not good when we have java developers in all different levels (from jr. do sr) working with cloudstack. It also makes us lose the type safety, which might also harm the exception handling if not done well. In addition, if we need to refactor something, the IDE is no longer going to do few things because the refection code cannot be found. If someone will need to extend the LibvirtComputingResource that would be no problem with the approach I’m using. The CitrixResourceBase also has quite few sub-classes and it works just fine. I will document on the wiki page how it should be done when sub-classing the LibvirtComputingResource class. In a quick note/snippet, one would do: public class EkhoComputingResource extends LibvirtComputingResource { @Override public Answer executeRequest(final Command cmd) { final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); try { return wrapper.execute(cmd, this); } catch (final Exception e) { return Answer.createUnsupportedCommandAnswer(cmd); } } } In the flyweight where I keep the wrapper we could have (): final HashtableClass? extends Command, CommandWrapper linbvirtCommands = new HashtableClass? extends Command, CommandWrapper(); linbvirtCommands.put(StopCommand.class, new LibvirtStopCommandWrapper()); final HashtableClass? extends Command, CommandWrapper ekhoCommands = new HashtableClass? extends Command, CommandWrapper(); linbvirtCommands.put(StopCommand.class, new EkhoStopCommandWrapper()); resources.put(LibvirtComputingResource.class, linbvirtCommands); resources.put(EkhoComputingResource.class, ekhoCommands); But that is needed only if the StopCommand has a different behaviour for the EkhoComputingResource. Once a better version of the documentation is on the wiki, I will let you know. On other matters, I’m also adding unit tests for all the changes. We already went from 4% to 13.6% coverage in the KVM hypervisor plugin. The code I already refactored has 56% of coverage. You can see all the commits here: https://github.com/schubergphilis/cloudstack/tree/refactor/libvirt_resource Cheers, Wilder On 23 Apr 2015, at 17:26, Marcus shadow...@gmail.commailto:shadow...@gmail.com wrote: Great to see someone working on it. What sorts of roadblocks came out of reflection? How does the wrapper design solve the pluggability issue? This is pretty important to me, since I've worked with several companies now that end up subclassing LibvirtComputingResource in order to handle their own Commands on the hypervisor from their server-side plugins, and changing their 'resource' to that in agent.properties. Since the main agent class needs to be set at agent join, this is harder to manage than it should be. I mentioned the reflection model because that's how I tend to handle the commands when subclassing LibvirtComputingResource. I haven't had any problems with it, but then again I haven't tried to refactor 5500 lines into that model, either. On Thu, Apr 23, 2015 at 1:17 AM, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.com wrote: Hi Marcus, I like the annotation idea, but reflection is trick because it hides some information about the code. Please, have a look at the CitrixResourceBase after the refactor I did. It became quite smaller and test coverage was improved. URL: https://cwiki.apache.org/confluence/display/CLOUDSTACK/Refactoring+XenServer+Hypervisor+Plugin The same patter is being about to Libvirt stuff. The coverage on the KVM hypervisor plugin already went from 4 to 10.5% after refactoring 6 commands Cheers, Wilder On 22 Apr 2015, at 23:06, Marcus shadow...@gmail.com wrote: Kind of a tangent, but I'd actually like to see some work done to clean up LibvirtComputing resource. One model I've prototyped that seems to work is to create an annotation, such as 'KVMCommandExecutor', with a 'handles' property. With this annotation, you implement a class that handles, e.g. StartCommand, etc. Then in LibvirtComputingResource, the 'configure' method fetches all of these executors via reflection and stores them in an object. Then, instead of having all of the 'instanceof' lines in LibvirtComputingResource, the executeRequest method fetches the executor that handles the incoming command and runs it. I think this would break up LibvirtComputingResource into smaller, more testable and manageable chunks, and force things like config and utility methods to move to a more sane location, as well. As a bonus, this model makes things pluggable. Someone could ship KVM plugin code containing standalone command executors that are discovered at runtime
Re: [DISCUSS] Moving to Java 8
Ok. I wish I could share some code, because it isn't really as big of a deal as it sounds from your reasoning. It is literally just 3 lines on startup that fetch anything with the '@AgentExecutor' annotation and stores it in a hash whose key is the value from @AgentExecutor's 'handles' property. Then when a *Command comes it it is passed to the appropriate Executor class. Looking at CitrixRequestWrapper, the 3 lines I mention are almost identical in function to your init method, just that it uses the annotation to find all of the commands, rather than hardcoding them. We use the same annotation design for the api side of the code on the management server, which allows the api commands to be easier to write and self-contained (you don't have to update other code to add a new api call). It makes things easier for novice developers. This implementation is no less typesafe than the previous design (the one with all of the instanceof). It didn't require any casting or warning suppression, either, as the wrapper does. Extending LibvirtComputingResource is not ideal, and doesn't work if multiple third parties are involved. Granted, there hasn't been a lot of demand for this, nevertheless it's particularly important for KVM, where the Command classes are executed on the hypervisor it's not really feasible to just dump the code in your management server-side plugin like some plugins do. In reviewing the code, the two implementations are really very close. If you just updated init to fetch the wrappers based on either an annotation or the class they extend, or something along those lines so this method doesn't have to be edited every time a command is added, that would be more or less the same thing. The the KVM agent would be pluggable like the management server side is. On Thu, Apr 30, 2015 at 12:55 PM, Wilder Rodrigues wrodrig...@schubergphilis.com wrote: Hi Marcus, Apologies for taking so much time to reply to your email, but was, and still am, quite busy. :) I would only use reflection if that was the only way to do it. The use of reflection usually makes the code more complex, which is not good when we have java developers in all different levels (from jr. do sr) working with cloudstack. It also makes us lose the type safety, which might also harm the exception handling if not done well. In addition, if we need to refactor something, the IDE is no longer going to do few things because the refection code cannot be found. If someone will need to extend the LibvirtComputingResource that would be no problem with the approach I’m using. The CitrixResourceBase also has quite few sub-classes and it works just fine. I will document on the wiki page how it should be done when sub-classing the LibvirtComputingResource class. In a quick note/snippet, one would do: public class EkhoComputingResource extends LibvirtComputingResource { @Override public Answer executeRequest(final Command cmd) { final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); try { return wrapper.execute(cmd, this); } catch (final Exception e) { return Answer.createUnsupportedCommandAnswer(cmd); } } } In the flyweight where I keep the wrapper we could have (): final HashtableClass? extends Command, CommandWrapper linbvirtCommands = new HashtableClass? extends Command, CommandWrapper(); linbvirtCommands.put(StopCommand.class, new LibvirtStopCommandWrapper()); final HashtableClass? extends Command, CommandWrapper ekhoCommands = new HashtableClass? extends Command, CommandWrapper(); linbvirtCommands.put(StopCommand.class, new EkhoStopCommandWrapper()); resources.put(LibvirtComputingResource.class, linbvirtCommands); resources.put(EkhoComputingResource.class, ekhoCommands); But that is needed only if the StopCommand has a different behaviour for the EkhoComputingResource. Once a better version of the documentation is on the wiki, I will let you know. On other matters, I’m also adding unit tests for all the changes. We already went from 4% to 13.6% coverage in the KVM hypervisor plugin. The code I already refactored has 56% of coverage. You can see all the commits here: https://github.com/schubergphilis/cloudstack/tree/refactor/libvirt_resource Cheers, Wilder On 23 Apr 2015, at 17:26, Marcus shadow...@gmail.com wrote: Great to see someone working on it. What sorts of roadblocks came out of reflection? How does the wrapper design solve the pluggability issue? This is pretty important to me, since I've worked with several companies now that end up subclassing LibvirtComputingResource in order to handle their own Commands on the hypervisor from their server-side plugins, and changing their 'resource' to that in agent.properties. Since the main agent class needs to be set at agent join, this is harder to manage than it should be. I
Re: [DISCUSS] Moving to Java 8
Hi Marcus, Thanks for the email… I’m always in for improvements. But why can’t you share the code? Few points below: 1. I added an subclassing example of LibvirtComputingResource because you mentioned it in a previous email: On 23 Apr 2015, at 17:26, Marcus shadow...@gmail.commailto:shadow...@gmail.com wrote: I mentioned the reflection model because that's how I tend to handle the commands when subclassing LibvirtComputingResource. 2. Current situation with LibvirtComputingResource on Master is: a. 67 IFs b. 67 private/protected methods that are used only there c. If a new Command is added it means we will have a new IF and a new private method e. Maintenance is hell, test is close to zero and code quality is below expectations That being said, the main idea with the refactor is to change structure only, not behaviour. So what I’m doing is to simply move the code out the LibvirtCompRes and write tests for it, keeping the behaviour the same - to be done in a next phase. If you look at the changes you will see that some wrappers are already 100% covered. However, some others have 4% or 8% (not that much though). I would like to refactor that as well, but that could change behaviour (mentioned above) which I don’t want to touch now. 3. With the new situation: a. No IFs b. All methods wrapped by other classes (command wrappers) - loosely coupled, easier to test and maintain c. If a new Command is added we would have to add a command wrapper and 1 line in the request wrapper implementation ( I know, it hurts you a bit) - but please bear with me for the good news. 4. the warnings are due to that: HashtableClass? extends Command, CommandWrapper() No big deal. As I understood from your first paragraph we would have to annotated the commands classes, right? I mean, all of them. That’s something I wouldn’t do in this phase, to be honest. It might seem harmless to do, but I like to break things down a bit and have more isolation in my changes. What’s next: I will finish the refactor with the request wrapper as it is. For me it is no problem do add the lines now and remove them in 1 week. Most of the work is concentrated in the tests, which I’m trying as hard as I can to get them in the best way possible. Once it’s done and pushed to master, I will analyse what we would need to apply the annotation. But before I go to bring the kids to school, just one question: a. The “handle” value, in the annotation, would have the wrapper class that would be used for that command, right? Now let’s get 1 command as example: CheckHealthCommand. Its wrapper implementation differs per hypervisor (just like all the other wrapper commands do). I’m not taking the time to really think about it now, but how would we annotated the different wrappers per command? Thanks again for your time. Cheers, Wilder On 30 Apr 2015, at 22:52, Marcus shadow...@gmail.commailto:shadow...@gmail.com wrote: Ok. I wish I could share some code, because it isn't really as big of a deal as it sounds from your reasoning. It is literally just 3 lines on startup that fetch anything with the '@AgentExecutor' annotation and stores it in a hash whose key is the value from @AgentExecutor's 'handles' property. Then when a *Command comes it it is passed to the appropriate Executor class. Looking at CitrixRequestWrapper, the 3 lines I mention are almost identical in function to your init method, just that it uses the annotation to find all of the commands, rather than hardcoding them. We use the same annotation design for the api side of the code on the management server, which allows the api commands to be easier to write and self-contained (you don't have to update other code to add a new api call). It makes things easier for novice developers. This implementation is no less typesafe than the previous design (the one with all of the instanceof). It didn't require any casting or warning suppression, either, as the wrapper does. Extending LibvirtComputingResource is not ideal, and doesn't work if multiple third parties are involved. Granted, there hasn't been a lot of demand for this, nevertheless it's particularly important for KVM, where the Command classes are executed on the hypervisor it's not really feasible to just dump the code in your management server-side plugin like some plugins do. In reviewing the code, the two implementations are really very close. If you just updated init to fetch the wrappers based on either an annotation or the class they extend, or something along those lines so this method doesn't have to be edited every time a command is added, that would be more or less the same thing. The the KVM agent would be pluggable like the management server side is. On Thu, Apr 30, 2015 at 12:55 PM, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.com wrote: Hi Marcus, Apologies for taking so much time to reply to your email, but was, and still am, quite busy. :) I
Re: [DISCUSS] Moving to Java 8
Great to see someone working on it. What sorts of roadblocks came out of reflection? How does the wrapper design solve the pluggability issue? This is pretty important to me, since I've worked with several companies now that end up subclassing LibvirtComputingResource in order to handle their own Commands on the hypervisor from their server-side plugins, and changing their 'resource' to that in agent.properties. Since the main agent class needs to be set at agent join, this is harder to manage than it should be. I mentioned the reflection model because that's how I tend to handle the commands when subclassing LibvirtComputingResource. I haven't had any problems with it, but then again I haven't tried to refactor 5500 lines into that model, either. On Thu, Apr 23, 2015 at 1:17 AM, Wilder Rodrigues wrodrig...@schubergphilis.com wrote: Hi Marcus, I like the annotation idea, but reflection is trick because it hides some information about the code. Please, have a look at the CitrixResourceBase after the refactor I did. It became quite smaller and test coverage was improved. URL: https://cwiki.apache.org/confluence/display/CLOUDSTACK/Refactoring+XenServer+Hypervisor+Plugin The same patter is being about to Libvirt stuff. The coverage on the KVM hypervisor plugin already went from 4 to 10.5% after refactoring 6 commands Cheers, Wilder On 22 Apr 2015, at 23:06, Marcus shadow...@gmail.com wrote: Kind of a tangent, but I'd actually like to see some work done to clean up LibvirtComputing resource. One model I've prototyped that seems to work is to create an annotation, such as 'KVMCommandExecutor', with a 'handles' property. With this annotation, you implement a class that handles, e.g. StartCommand, etc. Then in LibvirtComputingResource, the 'configure' method fetches all of these executors via reflection and stores them in an object. Then, instead of having all of the 'instanceof' lines in LibvirtComputingResource, the executeRequest method fetches the executor that handles the incoming command and runs it. I think this would break up LibvirtComputingResource into smaller, more testable and manageable chunks, and force things like config and utility methods to move to a more sane location, as well. As a bonus, this model makes things pluggable. Someone could ship KVM plugin code containing standalone command executors that are discovered at runtime for things they need to run at the hypervisor level. On Tue, Apr 21, 2015 at 6:27 AM, Wilder Rodrigues wrodrig...@schubergphilis.com wrote: Hi all, Yesterday I started working on the LibvirtComputingResource class in order to apply the same patterns I used in the CitrixResourceBase + add more unit tests to it After 10 hours of work I got a bit stuck with the 1st test, which would cover the refactored LibvirtStopCommandWrapper. Why did I get stuck? The class used a few static methods that call native libraries, which I would like to mock. However, when writing the tests I faced problems with the current Mockito/PowerMock we are using: they are simply not enough for the task. What did I do then? I added a dependency to EasyMock and PowerMock-EasyMock API. It worked almost fine, but I had to add a “-noverify” to both my Eclipse Runtime configuration and also to the cloud-plugin-hypervisor-kvm/pom.xml file. I agree that’s not nice, but was my first attempt of getting it to work. After trying to first full build I faced more problems related to ClassDefNotFoundExpcetion which were complaining about Mockito classes. I then found out that adding the PowerMockRunner to all the tests classes was going to be a heavy burden and would also mess up future changes (e.g. the -noverify flag was removed from Java 8, thus adding it now would be a problem soon). Now that the first 2 paragraphs explain a bit about the problem, let’s get to the solution: Java 8 The VerifyError that I was getting was due to the use of the latest EasyMock release (3.3.1). I tried to downgrade it to 3.1/3.2 but it also did not work. My decision: do not refactor if the proper tests cannot be added. This left me with one action: migrate to Java 8. There were mentions about Java 8 in february[1] and now I will put some energy in making it happen. What is your opinion on it? Thanks in advance. Cheers, Wilder http://mail-archives.apache.org/mod_mbox/cloudstack-dev/201502.mbox/%3c54eef6be.5040...@shapeblue.com%3Ehttp://mail-archives.apache.org/mod_mbox/cloudstack-dev/201502.mbox/54eef6be.5040...@shapeblue.com
Re: [DISCUSS] Moving to Java 8
Hi Marcus, I like the annotation idea, but reflection is trick because it hides some information about the code. Please, have a look at the CitrixResourceBase after the refactor I did. It became quite smaller and test coverage was improved. URL: https://cwiki.apache.org/confluence/display/CLOUDSTACK/Refactoring+XenServer+Hypervisor+Plugin The same patter is being about to Libvirt stuff. The coverage on the KVM hypervisor plugin already went from 4 to 10.5% after refactoring 6 commands Cheers, Wilder On 22 Apr 2015, at 23:06, Marcus shadow...@gmail.commailto:shadow...@gmail.com wrote: Kind of a tangent, but I'd actually like to see some work done to clean up LibvirtComputing resource. One model I've prototyped that seems to work is to create an annotation, such as 'KVMCommandExecutor', with a 'handles' property. With this annotation, you implement a class that handles, e.g. StartCommand, etc. Then in LibvirtComputingResource, the 'configure' method fetches all of these executors via reflection and stores them in an object. Then, instead of having all of the 'instanceof' lines in LibvirtComputingResource, the executeRequest method fetches the executor that handles the incoming command and runs it. I think this would break up LibvirtComputingResource into smaller, more testable and manageable chunks, and force things like config and utility methods to move to a more sane location, as well. As a bonus, this model makes things pluggable. Someone could ship KVM plugin code containing standalone command executors that are discovered at runtime for things they need to run at the hypervisor level. On Tue, Apr 21, 2015 at 6:27 AM, Wilder Rodrigues wrodrig...@schubergphilis.commailto:wrodrig...@schubergphilis.com wrote: Hi all, Yesterday I started working on the LibvirtComputingResource class in order to apply the same patterns I used in the CitrixResourceBase + add more unit tests to it After 10 hours of work I got a bit stuck with the 1st test, which would cover the refactored LibvirtStopCommandWrapper. Why did I get stuck? The class used a few static methods that call native libraries, which I would like to mock. However, when writing the tests I faced problems with the current Mockito/PowerMock we are using: they are simply not enough for the task. What did I do then? I added a dependency to EasyMock and PowerMock-EasyMock API. It worked almost fine, but I had to add a “-noverify” to both my Eclipse Runtime configuration and also to the cloud-plugin-hypervisor-kvm/pom.xml file. I agree that’s not nice, but was my first attempt of getting it to work. After trying to first full build I faced more problems related to ClassDefNotFoundExpcetion which were complaining about Mockito classes. I then found out that adding the PowerMockRunner to all the tests classes was going to be a heavy burden and would also mess up future changes (e.g. the -noverify flag was removed from Java 8, thus adding it now would be a problem soon). Now that the first 2 paragraphs explain a bit about the problem, let’s get to the solution: Java 8 The VerifyError that I was getting was due to the use of the latest EasyMock release (3.3.1). I tried to downgrade it to 3.1/3.2 but it also did not work. My decision: do not refactor if the proper tests cannot be added. This left me with one action: migrate to Java 8. There were mentions about Java 8 in february[1] and now I will put some energy in making it happen. What is your opinion on it? Thanks in advance. Cheers, Wilder http://mail-archives.apache.org/mod_mbox/cloudstack-dev/201502.mbox/%3c54eef6be.5040...@shapeblue.com%3Ehttp://mail-archives.apache.org/mod_mbox/cloudstack-dev/201502.mbox/54eef6be.5040...@shapeblue.comhttp://mail-archives.apache.org/mod_mbox/cloudstack-dev/201502.mbox/%3c54eef6be.5040...@shapeblue.com%3E%3Chttp://mail-archives.apache.org/mod_mbox/cloudstack-dev/201502.mbox/%3c54eef6be.5040...@shapeblue.com%3E%3E
Re: [DISCUSS] Moving to Java 8
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 04/22/2015 02:13 PM, Wilder Rodrigues wrote: Hi Wido, Thanks for the reply and making a good point concerning Ubuntu 14.04. Besides the difficulty in writing testes without increasing even more out technical dept, another point on the Java 8 platform is the EOL (end of this month) of Java 1.7. Yes, I'm aware of that. So that makes this situation difficult. There is a bug open for backporting OpenJDK 8 to Ubuntu 14.04: https://bugs.launchpad.net/ubuntu/+source/openjdk-8/+bug/1341628 More votes there would really help. For now I created a ticket on Apache Jira to keep track of it: https://issues.apache.org/jira/browse/CLOUDSTACK-8397. Could you please have a look and let me know if the content of the ticket is appropriate? We will start a new sprint in 1 week and will take some time to discuss what to do and when. Will keep the community updated on that matter. Thanks a gain. Cheers, Wilder On 21 Apr 2015, at 15:49, Wido den Hollander w...@widodh.nlmailto:w...@widodh.nl wrote: On 04/21/2015 03:27 PM, Wilder Rodrigues wrote: Hi all, Yesterday I started working on the LibvirtComputingResource class in order to apply the same patterns I used in the CitrixResourceBase + add more unit tests to it After 10 hours of work I got a bit stuck with the 1st test, which would cover the refactored LibvirtStopCommandWrapper. Why did I get stuck? The class used a few static methods that call native libraries, which I would like to mock. However, when writing the tests I faced problems with the current Mockito/PowerMock we are using: they are simply not enough for the task. What did I do then? I added a dependency to EasyMock and PowerMock-EasyMock API. It worked almost fine, but I had to add a “-noverify” to both my Eclipse Runtime configuration and also to the cloud-plugin-hypervisor-kvm/pom.xml file. I agree that’s not nice, but was my first attempt of getting it to work. After trying to first full build I faced more problems related to ClassDefNotFoundExpcetion which were complaining about Mockito classes. I then found out that adding the PowerMockRunner to all the tests classes was going to be a heavy burden and would also mess up future changes (e.g. the -noverify flag was removed from Java 8, thus adding it now would be a problem soon). Now that the first 2 paragraphs explain a bit about the problem, let’s get to the solution: Java 8 The VerifyError that I was getting was due to the use of the latest EasyMock release (3.3.1). I tried to downgrade it to 3.1/3.2 but it also did not work. My decision: do not refactor if the proper tests cannot be added. This left me with one action: migrate to Java 8. There were mentions about Java 8 in february[1] and now I will put some energy in making it happen. What is your opinion on it? I'm not against it technically, but practically I am. Ubuntu 14.04 does not ship a Java 8 JRE in the repositories. CentOS 7 has java-1.8.0-openjdk.x86_64 available, so it would work there. But Ubuntu is also widely used with CloudStack, so those users couldn't use CloudStack without any additional repositories. Since that isn't easy I would vote -1 on this if it came that far. Wido Thanks in advance. Cheers, Wilder http://mail-archives.apache.org/mod_mbox/cloudstack-dev/201502.mbox/%3 c54eef6be.5040...@shapeblue.commailto:c54eef6be.5040...@shapeblue.com% 3Ehttp://mail-archives.apache.org/mod_m box/cloudstack-dev/201502.mbox/54eef6be.5040...@shapeblue.commailto: 54eef6be.5040...@shapeblue.com -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQIcBAEBAgAGBQJVN5dgAAoJEAGbWC3bPspCzX8P/0o8V8ZZPF+mOiXwfvZxOoqn Xtb084SajpLvB4KFT207FecJ6rKJyJiSXZSW6esj1F5OBzoDzF30vWHityvvatCA LsY5zCj2LF01itmjK8SVXuuwK8sSINycJPu2jJVotYr4ooPM1pHJjv/UnQfrUgp3 yv8vT3VKhrPLkGOIIcRR8zmIPH6qtgTf/ILBsc9hUrkvYgfmReH1dkeQY4gid3TZ sHGrnYHby2SgW+9KbuEfdvOrHvItYbJpRWz6W3R6l+DQeZxMt/pMbZRXEM4LXkuL /t450iwybpMZzwwnPYqu0TjTjSI0AFZp9gq+obygEnbDsbCXMuUKRHNymCdUZH3r hldfP1dmAwPEjJ0Z9PgybvmitaAqvUg80BeS7iS/6SPulGx6hFiafrdCgjdQ7bxM qN4nGcFIwxmzphhljnLARxRxl2/50KuZTYmC3XmyfbrUYX+BiDW8FoMbCd5qJJyC r48w2gPQW52HyVckM532PFpMWah7If8Q0Ee9w0JrTEF3RRkMQ7NySUvK0Y9er8ay D6fmw5szuWWNF7bSC6wNx5bY7EyNGp7rDa1Ki4f4G7chh6m16yhYSQ0fLBjro6fw sFbs7EKAa9iCsyHbPQBpz3IoqkSbCknnHbXAInQOyybLKendwZ2f9LVA8K0GFlHH uMsBvF/FH1bnVg+oX8N3 =oFqF -END PGP SIGNATURE-
Re: [DISCUSS] Moving to Java 8
Kind of a tangent, but I'd actually like to see some work done to clean up LibvirtComputing resource. One model I've prototyped that seems to work is to create an annotation, such as 'KVMCommandExecutor', with a 'handles' property. With this annotation, you implement a class that handles, e.g. StartCommand, etc. Then in LibvirtComputingResource, the 'configure' method fetches all of these executors via reflection and stores them in an object. Then, instead of having all of the 'instanceof' lines in LibvirtComputingResource, the executeRequest method fetches the executor that handles the incoming command and runs it. I think this would break up LibvirtComputingResource into smaller, more testable and manageable chunks, and force things like config and utility methods to move to a more sane location, as well. As a bonus, this model makes things pluggable. Someone could ship KVM plugin code containing standalone command executors that are discovered at runtime for things they need to run at the hypervisor level. On Tue, Apr 21, 2015 at 6:27 AM, Wilder Rodrigues wrodrig...@schubergphilis.com wrote: Hi all, Yesterday I started working on the LibvirtComputingResource class in order to apply the same patterns I used in the CitrixResourceBase + add more unit tests to it After 10 hours of work I got a bit stuck with the 1st test, which would cover the refactored LibvirtStopCommandWrapper. Why did I get stuck? The class used a few static methods that call native libraries, which I would like to mock. However, when writing the tests I faced problems with the current Mockito/PowerMock we are using: they are simply not enough for the task. What did I do then? I added a dependency to EasyMock and PowerMock-EasyMock API. It worked almost fine, but I had to add a “-noverify” to both my Eclipse Runtime configuration and also to the cloud-plugin-hypervisor-kvm/pom.xml file. I agree that’s not nice, but was my first attempt of getting it to work. After trying to first full build I faced more problems related to ClassDefNotFoundExpcetion which were complaining about Mockito classes. I then found out that adding the PowerMockRunner to all the tests classes was going to be a heavy burden and would also mess up future changes (e.g. the -noverify flag was removed from Java 8, thus adding it now would be a problem soon). Now that the first 2 paragraphs explain a bit about the problem, let’s get to the solution: Java 8 The VerifyError that I was getting was due to the use of the latest EasyMock release (3.3.1). I tried to downgrade it to 3.1/3.2 but it also did not work. My decision: do not refactor if the proper tests cannot be added. This left me with one action: migrate to Java 8. There were mentions about Java 8 in february[1] and now I will put some energy in making it happen. What is your opinion on it? Thanks in advance. Cheers, Wilder http://mail-archives.apache.org/mod_mbox/cloudstack-dev/201502.mbox/%3c54eef6be.5040...@shapeblue.com%3Ehttp://mail-archives.apache.org/mod_mbox/cloudstack-dev/201502.mbox/54eef6be.5040...@shapeblue.com
Re: [DISCUSS] Moving to Java 8
I agree with Wido. The first thing that came to mind is legacy installs of CentOS that have java6,7 but not java8. I'd prefer to wait until we can reasonably say that Ubuntu 12.04 and CentOS6.x are artifacts of the past that people should not be running (maybe 6-12mo past LTS, or something). Unless there's a real necessity to switch, I'd say we don't. I think the distributions which have shipped these versions, if they don't provide a newer version in a point release will contribute to support/patch the version of openjdk they do ship in their LTS. On Wed, Apr 22, 2015 at 1:44 PM, Laszlo Hornyak laszlo.horn...@gmail.com wrote: Is java 1.8 on the roadmap for ubuntu 14? It does not seem to make sense to have a LTS supported until 2019 with a JDK no longer supported. On Wed, Apr 22, 2015 at 2:43 PM, Wido den Hollander w...@widodh.nl wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 04/22/2015 02:13 PM, Wilder Rodrigues wrote: Hi Wido, Thanks for the reply and making a good point concerning Ubuntu 14.04. Besides the difficulty in writing testes without increasing even more out technical dept, another point on the Java 8 platform is the EOL (end of this month) of Java 1.7. Yes, I'm aware of that. So that makes this situation difficult. There is a bug open for backporting OpenJDK 8 to Ubuntu 14.04: https://bugs.launchpad.net/ubuntu/+source/openjdk-8/+bug/1341628 More votes there would really help. For now I created a ticket on Apache Jira to keep track of it: https://issues.apache.org/jira/browse/CLOUDSTACK-8397. Could you please have a look and let me know if the content of the ticket is appropriate? We will start a new sprint in 1 week and will take some time to discuss what to do and when. Will keep the community updated on that matter. Thanks a gain. Cheers, Wilder On 21 Apr 2015, at 15:49, Wido den Hollander w...@widodh.nlmailto:w...@widodh.nl wrote: On 04/21/2015 03:27 PM, Wilder Rodrigues wrote: Hi all, Yesterday I started working on the LibvirtComputingResource class in order to apply the same patterns I used in the CitrixResourceBase + add more unit tests to it After 10 hours of work I got a bit stuck with the 1st test, which would cover the refactored LibvirtStopCommandWrapper. Why did I get stuck? The class used a few static methods that call native libraries, which I would like to mock. However, when writing the tests I faced problems with the current Mockito/PowerMock we are using: they are simply not enough for the task. What did I do then? I added a dependency to EasyMock and PowerMock-EasyMock API. It worked almost fine, but I had to add a “-noverify” to both my Eclipse Runtime configuration and also to the cloud-plugin-hypervisor-kvm/pom.xml file. I agree that’s not nice, but was my first attempt of getting it to work. After trying to first full build I faced more problems related to ClassDefNotFoundExpcetion which were complaining about Mockito classes. I then found out that adding the PowerMockRunner to all the tests classes was going to be a heavy burden and would also mess up future changes (e.g. the -noverify flag was removed from Java 8, thus adding it now would be a problem soon). Now that the first 2 paragraphs explain a bit about the problem, let’s get to the solution: Java 8 The VerifyError that I was getting was due to the use of the latest EasyMock release (3.3.1). I tried to downgrade it to 3.1/3.2 but it also did not work. My decision: do not refactor if the proper tests cannot be added. This left me with one action: migrate to Java 8. There were mentions about Java 8 in february[1] and now I will put some energy in making it happen. What is your opinion on it? I'm not against it technically, but practically I am. Ubuntu 14.04 does not ship a Java 8 JRE in the repositories. CentOS 7 has java-1.8.0-openjdk.x86_64 available, so it would work there. But Ubuntu is also widely used with CloudStack, so those users couldn't use CloudStack without any additional repositories. Since that isn't easy I would vote -1 on this if it came that far. Wido Thanks in advance. Cheers, Wilder http://mail-archives.apache.org/mod_mbox/cloudstack-dev/201502.mbox/%3 c54eef6be.5040...@shapeblue.commailto:c54eef6be.5040...@shapeblue.com% 3Ehttp://mail-archives.apache.org/mod_m box/cloudstack-dev/201502.mbox/54eef6be.5040...@shapeblue.commailto: 54eef6be.5040...@shapeblue.com -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQIcBAEBAgAGBQJVN5dgAAoJEAGbWC3bPspCzX8P/0o8V8ZZPF+mOiXwfvZxOoqn Xtb084SajpLvB4KFT207FecJ6rKJyJiSXZSW6esj1F5OBzoDzF30vWHityvvatCA LsY5zCj2LF01itmjK8SVXuuwK8sSINycJPu2jJVotYr4ooPM1pHJjv/UnQfrUgp3 yv8vT3VKhrPLkGOIIcRR8zmIPH6qtgTf/ILBsc9hUrkvYgfmReH1dkeQY4gid3TZ sHGrnYHby2SgW+9KbuEfdvOrHvItYbJpRWz6W3R6l+DQeZxMt/pMbZRXEM4LXkuL
Re: [DISCUSS] Moving to Java 8
Is java 1.8 on the roadmap for ubuntu 14? It does not seem to make sense to have a LTS supported until 2019 with a JDK no longer supported. On Wed, Apr 22, 2015 at 2:43 PM, Wido den Hollander w...@widodh.nl wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 04/22/2015 02:13 PM, Wilder Rodrigues wrote: Hi Wido, Thanks for the reply and making a good point concerning Ubuntu 14.04. Besides the difficulty in writing testes without increasing even more out technical dept, another point on the Java 8 platform is the EOL (end of this month) of Java 1.7. Yes, I'm aware of that. So that makes this situation difficult. There is a bug open for backporting OpenJDK 8 to Ubuntu 14.04: https://bugs.launchpad.net/ubuntu/+source/openjdk-8/+bug/1341628 More votes there would really help. For now I created a ticket on Apache Jira to keep track of it: https://issues.apache.org/jira/browse/CLOUDSTACK-8397. Could you please have a look and let me know if the content of the ticket is appropriate? We will start a new sprint in 1 week and will take some time to discuss what to do and when. Will keep the community updated on that matter. Thanks a gain. Cheers, Wilder On 21 Apr 2015, at 15:49, Wido den Hollander w...@widodh.nlmailto:w...@widodh.nl wrote: On 04/21/2015 03:27 PM, Wilder Rodrigues wrote: Hi all, Yesterday I started working on the LibvirtComputingResource class in order to apply the same patterns I used in the CitrixResourceBase + add more unit tests to it After 10 hours of work I got a bit stuck with the 1st test, which would cover the refactored LibvirtStopCommandWrapper. Why did I get stuck? The class used a few static methods that call native libraries, which I would like to mock. However, when writing the tests I faced problems with the current Mockito/PowerMock we are using: they are simply not enough for the task. What did I do then? I added a dependency to EasyMock and PowerMock-EasyMock API. It worked almost fine, but I had to add a “-noverify” to both my Eclipse Runtime configuration and also to the cloud-plugin-hypervisor-kvm/pom.xml file. I agree that’s not nice, but was my first attempt of getting it to work. After trying to first full build I faced more problems related to ClassDefNotFoundExpcetion which were complaining about Mockito classes. I then found out that adding the PowerMockRunner to all the tests classes was going to be a heavy burden and would also mess up future changes (e.g. the -noverify flag was removed from Java 8, thus adding it now would be a problem soon). Now that the first 2 paragraphs explain a bit about the problem, let’s get to the solution: Java 8 The VerifyError that I was getting was due to the use of the latest EasyMock release (3.3.1). I tried to downgrade it to 3.1/3.2 but it also did not work. My decision: do not refactor if the proper tests cannot be added. This left me with one action: migrate to Java 8. There were mentions about Java 8 in february[1] and now I will put some energy in making it happen. What is your opinion on it? I'm not against it technically, but practically I am. Ubuntu 14.04 does not ship a Java 8 JRE in the repositories. CentOS 7 has java-1.8.0-openjdk.x86_64 available, so it would work there. But Ubuntu is also widely used with CloudStack, so those users couldn't use CloudStack without any additional repositories. Since that isn't easy I would vote -1 on this if it came that far. Wido Thanks in advance. Cheers, Wilder http://mail-archives.apache.org/mod_mbox/cloudstack-dev/201502.mbox/%3 c54eef6be.5040...@shapeblue.commailto:c54eef6be.5040...@shapeblue.com% 3Ehttp://mail-archives.apache.org/mod_m box/cloudstack-dev/201502.mbox/54eef6be.5040...@shapeblue.commailto: 54eef6be.5040...@shapeblue.com -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQIcBAEBAgAGBQJVN5dgAAoJEAGbWC3bPspCzX8P/0o8V8ZZPF+mOiXwfvZxOoqn Xtb084SajpLvB4KFT207FecJ6rKJyJiSXZSW6esj1F5OBzoDzF30vWHityvvatCA LsY5zCj2LF01itmjK8SVXuuwK8sSINycJPu2jJVotYr4ooPM1pHJjv/UnQfrUgp3 yv8vT3VKhrPLkGOIIcRR8zmIPH6qtgTf/ILBsc9hUrkvYgfmReH1dkeQY4gid3TZ sHGrnYHby2SgW+9KbuEfdvOrHvItYbJpRWz6W3R6l+DQeZxMt/pMbZRXEM4LXkuL /t450iwybpMZzwwnPYqu0TjTjSI0AFZp9gq+obygEnbDsbCXMuUKRHNymCdUZH3r hldfP1dmAwPEjJ0Z9PgybvmitaAqvUg80BeS7iS/6SPulGx6hFiafrdCgjdQ7bxM qN4nGcFIwxmzphhljnLARxRxl2/50KuZTYmC3XmyfbrUYX+BiDW8FoMbCd5qJJyC r48w2gPQW52HyVckM532PFpMWah7If8Q0Ee9w0JrTEF3RRkMQ7NySUvK0Y9er8ay D6fmw5szuWWNF7bSC6wNx5bY7EyNGp7rDa1Ki4f4G7chh6m16yhYSQ0fLBjro6fw sFbs7EKAa9iCsyHbPQBpz3IoqkSbCknnHbXAInQOyybLKendwZ2f9LVA8K0GFlHH uMsBvF/FH1bnVg+oX8N3 =oFqF -END PGP SIGNATURE- -- EOF
Re: [DISCUSS] Moving to Java 8
Hi Wido, Thanks for the reply and making a good point concerning Ubuntu 14.04. Besides the difficulty in writing testes without increasing even more out technical dept, another point on the Java 8 platform is the EOL (end of this month) of Java 1.7. For now I created a ticket on Apache Jira to keep track of it: https://issues.apache.org/jira/browse/CLOUDSTACK-8397. Could you please have a look and let me know if the content of the ticket is appropriate? We will start a new sprint in 1 week and will take some time to discuss what to do and when. Will keep the community updated on that matter. Thanks a gain. Cheers, Wilder On 21 Apr 2015, at 15:49, Wido den Hollander w...@widodh.nlmailto:w...@widodh.nl wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 04/21/2015 03:27 PM, Wilder Rodrigues wrote: Hi all, Yesterday I started working on the LibvirtComputingResource class in order to apply the same patterns I used in the CitrixResourceBase + add more unit tests to it After 10 hours of work I got a bit stuck with the 1st test, which would cover the refactored LibvirtStopCommandWrapper. Why did I get stuck? The class used a few static methods that call native libraries, which I would like to mock. However, when writing the tests I faced problems with the current Mockito/PowerMock we are using: they are simply not enough for the task. What did I do then? I added a dependency to EasyMock and PowerMock-EasyMock API. It worked almost fine, but I had to add a “-noverify” to both my Eclipse Runtime configuration and also to the cloud-plugin-hypervisor-kvm/pom.xml file. I agree that’s not nice, but was my first attempt of getting it to work. After trying to first full build I faced more problems related to ClassDefNotFoundExpcetion which were complaining about Mockito classes. I then found out that adding the PowerMockRunner to all the tests classes was going to be a heavy burden and would also mess up future changes (e.g. the -noverify flag was removed from Java 8, thus adding it now would be a problem soon). Now that the first 2 paragraphs explain a bit about the problem, let’s get to the solution: Java 8 The VerifyError that I was getting was due to the use of the latest EasyMock release (3.3.1). I tried to downgrade it to 3.1/3.2 but it also did not work. My decision: do not refactor if the proper tests cannot be added. This left me with one action: migrate to Java 8. There were mentions about Java 8 in february[1] and now I will put some energy in making it happen. What is your opinion on it? I'm not against it technically, but practically I am. Ubuntu 14.04 does not ship a Java 8 JRE in the repositories. CentOS 7 has java-1.8.0-openjdk.x86_64 available, so it would work there. But Ubuntu is also widely used with CloudStack, so those users couldn't use CloudStack without any additional repositories. Since that isn't easy I would vote -1 on this if it came that far. Wido Thanks in advance. Cheers, Wilder http://mail-archives.apache.org/mod_mbox/cloudstack-dev/201502.mbox/%3 c54eef6be.5040...@shapeblue.commailto:c54eef6be.5040...@shapeblue.com%3Ehttp://mail-archives.apache.org/mod_m box/cloudstack-dev/201502.mbox/54eef6be.5040...@shapeblue.commailto:54eef6be.5040...@shapeblue.com -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQIcBAEBAgAGBQJVNlV2AAoJEAGbWC3bPspCfYgQAMoXgekM6/NHXivIGLfbhekK ghf+Ll1EFku+9IdMUdaholO0bLz7TQ+DOphr+xikVNizqakSnYCSF5wjiKQYtc+g LCPvWhbp8UG4LjbjWdK80FPtbx0WH9OZg/XK097NQoFRNpchxRprpFoSSYJMAhqh WjDvwfgzCITw9pqC3jI8l3Sy/i1/yv7fFRv//w4vHOqRa+4urst+dXASGer83IAX zCJdp+SxOx2VnUDta32KJFc6CsV2rV4BW+5dPAhJad7mFd6EafngSWZRuJmjkbsq tSiULlUAluXSnZ86FQESDhffYJdp/fkjWbmZvSRYgE+Xn5cAyB02z9ukLeJktNgq 3UjIyINiU7182IXlrRUciZXXexc8jfYCaQ8wmgOjxmpPdih0zqWBa4RWA8VCt4SM Sw5BXO3hDjXljxrbeXs3AHSDPAslNoSDOEmKVeLYDdMWbO0Z7FvUurRuv4LKgOdE lofDGIcUBw++AE1DuF+vIU/Gnfo61oMzY2FPvXp4V+wHekf+uet8H4MPs0Y5PFDD 9HyewO+aWEC0w8LQVAIevP8pwbArv4zX65AtSWI2IKqmwQOpvIeuB2M7sdxgoNM+ M7IMdYBC3XyIcxqYwDvevlAM0PoeUhpYY9m6fpfirlhJMXS8nBEA1H45cQQBCVDz /uhF9CC8wnEQcb2cu+lF =Hhwq -END PGP SIGNATURE-
Re: [DISCUSS] Moving to Java 8
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 04/21/2015 03:27 PM, Wilder Rodrigues wrote: Hi all, Yesterday I started working on the LibvirtComputingResource class in order to apply the same patterns I used in the CitrixResourceBase + add more unit tests to it After 10 hours of work I got a bit stuck with the 1st test, which would cover the refactored LibvirtStopCommandWrapper. Why did I get stuck? The class used a few static methods that call native libraries, which I would like to mock. However, when writing the tests I faced problems with the current Mockito/PowerMock we are using: they are simply not enough for the task. What did I do then? I added a dependency to EasyMock and PowerMock-EasyMock API. It worked almost fine, but I had to add a “-noverify” to both my Eclipse Runtime configuration and also to the cloud-plugin-hypervisor-kvm/pom.xml file. I agree that’s not nice, but was my first attempt of getting it to work. After trying to first full build I faced more problems related to ClassDefNotFoundExpcetion which were complaining about Mockito classes. I then found out that adding the PowerMockRunner to all the tests classes was going to be a heavy burden and would also mess up future changes (e.g. the -noverify flag was removed from Java 8, thus adding it now would be a problem soon). Now that the first 2 paragraphs explain a bit about the problem, let’s get to the solution: Java 8 The VerifyError that I was getting was due to the use of the latest EasyMock release (3.3.1). I tried to downgrade it to 3.1/3.2 but it also did not work. My decision: do not refactor if the proper tests cannot be added. This left me with one action: migrate to Java 8. There were mentions about Java 8 in february[1] and now I will put some energy in making it happen. What is your opinion on it? I'm not against it technically, but practically I am. Ubuntu 14.04 does not ship a Java 8 JRE in the repositories. CentOS 7 has java-1.8.0-openjdk.x86_64 available, so it would work there. But Ubuntu is also widely used with CloudStack, so those users couldn't use CloudStack without any additional repositories. Since that isn't easy I would vote -1 on this if it came that far. Wido Thanks in advance. Cheers, Wilder http://mail-archives.apache.org/mod_mbox/cloudstack-dev/201502.mbox/%3 c54eef6be.5040...@shapeblue.com%3Ehttp://mail-archives.apache.org/mod_m box/cloudstack-dev/201502.mbox/54eef6be.5040...@shapeblue.com -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQIcBAEBAgAGBQJVNlV2AAoJEAGbWC3bPspCfYgQAMoXgekM6/NHXivIGLfbhekK ghf+Ll1EFku+9IdMUdaholO0bLz7TQ+DOphr+xikVNizqakSnYCSF5wjiKQYtc+g LCPvWhbp8UG4LjbjWdK80FPtbx0WH9OZg/XK097NQoFRNpchxRprpFoSSYJMAhqh WjDvwfgzCITw9pqC3jI8l3Sy/i1/yv7fFRv//w4vHOqRa+4urst+dXASGer83IAX zCJdp+SxOx2VnUDta32KJFc6CsV2rV4BW+5dPAhJad7mFd6EafngSWZRuJmjkbsq tSiULlUAluXSnZ86FQESDhffYJdp/fkjWbmZvSRYgE+Xn5cAyB02z9ukLeJktNgq 3UjIyINiU7182IXlrRUciZXXexc8jfYCaQ8wmgOjxmpPdih0zqWBa4RWA8VCt4SM Sw5BXO3hDjXljxrbeXs3AHSDPAslNoSDOEmKVeLYDdMWbO0Z7FvUurRuv4LKgOdE lofDGIcUBw++AE1DuF+vIU/Gnfo61oMzY2FPvXp4V+wHekf+uet8H4MPs0Y5PFDD 9HyewO+aWEC0w8LQVAIevP8pwbArv4zX65AtSWI2IKqmwQOpvIeuB2M7sdxgoNM+ M7IMdYBC3XyIcxqYwDvevlAM0PoeUhpYY9m6fpfirlhJMXS8nBEA1H45cQQBCVDz /uhF9CC8wnEQcb2cu+lF =Hhwq -END PGP SIGNATURE-