incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Min Chen <min.c...@citrix.com>
Subject Re: Duplicate BareMetalTemplateAdapter classes
Date Thu, 14 Feb 2013 17:26:48 GMT
This fix will come to 4.1 along with Kelven's fix of removing
auto-scanning, not through my feature branch merge since that feature
branch will only be merged to master later.

Thanks
-min

On 2/14/13 6:57 AM, "Chip Childers" <chip.childers@sungard.com> wrote:

>On Thu, Feb 14, 2013 at 11:24:26AM +0530, Rohit Yadav wrote:
>> Okay, but the fix is in vim51_win8, do you want to cherry pick on
>> master/4.1 now or will it come when you'll merge your branch on
>> master? If you do the latter, it won't be part of 4.1.
>
>It seems like a fix for this should be in 4.1, but please limit the
>scope of any 4.1 fix to something small and reasonable (not sure if the
>orig fix does that or not).
>
>> 
>> Regards.
>> 
>> On Thu, Feb 14, 2013 at 12:22 AM, Min Chen <min.chen@citrix.com> wrote:
>> > Rohit, it turned out that we were fixing the same bug yesterday:) I
>>also
>> > ran into template registration issue in vmware testing. I saw that you
>> > fixed that issue through modifying java code. After discussing with
>>Kelven
>> > and Alex, we think that a more standard way to fix is to address this
>>in
>> > componentContext.xml (nonossComponentContext.xml) to set name
>>property and
>> > remove @Component annotation from BareMetalTemplateAdapater and
>> > HyervisorTemplateAdapater (here our code has a typo here, this class
>> > should be named as HypervisorTemplateAdapter). You can see that fix
>>in my
>> > feature branch
>> > 
>>(https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=com
>>mi
>> > t;h=c0442e2556a7b10cfb363a2d4d55b7fb9c381297).  Kelven is cleaning up
>>code
>> > to use XML to load all component and remove auto-scanning, this bug
>>fix
>> > will be addressed there during his cleanup.
>> >
>> > Thanks
>> > -min
>> >
>> > On 2/13/13 10:35 AM, "Frank Zhang" <Frank.Zhang@citrix.com> wrote:
>> >
>> >>Thanks for filing this bug.
>> >>I am working on it recently, because new baremetal which is as a
>>single
>> >>plugin haven't been checked in when javelin refactoring. So there are
>> >>some old code which is not working still stay in source, I am cleaning
>> >>up/testing in my local source,
>> >>will close these bug when fixes are checked in
>> >>
>> >>From: rohityadav89@gmail.com [mailto:rohityadav89@gmail.com] On
>>Behalf Of
>> >>Rohit Yadav
>> >>Sent: Wednesday, February 13, 2013 2:01 AM
>> >>To: Frank Zhang
>> >>Cc: cloudstack
>> >>Subject: Duplicate BareMetalTemplateAdapter classes
>> >>
>> >>Hi Frank,
>> >>
>> >>Can you check why we have two BareMetalTemplateAdapter;
>> 
>>>>./plugins/hypervisors/baremetal/src/com/cloud/baremetal/manager/BareMet
>>>>alT
>> >>emplateAdapter.java (Found no usage across codebase, removable?)
>> >>./server/src/com/cloud/baremetal/BareMetalTemplateAdapter.java (only
>>this
>> >>one is used, as spring would instantiate only this one with @Component
>> >>annotation)
>> >>
>> >>And remove one which is redundant code? Found this while fixing
>> >>CLOUDSTACK-1237.
>> >>
>> >>Regards.
>> >>On Wed, Feb 13, 2013 at 3:26 PM, Rohit Yadav
>> >><bhaisaab@apache.org<mailto:bhaisaab@apache.org>> wrote:
>> >>This is an automatically generated e-mail. To reply, visit:
>> >>https://reviews.apache.org/r/9420/
>> >>
>> >>
>> >>
>> >>
>> >>This fix would have worked for Hypervisor but would have failed for
>> >>baremetal... if we fix like this, there may be other template adapters
>> >>whose class (simple) names. So, it was better to impl getName() for
>>all
>> >>implementing template adapters.
>> >>
>> >>
>> >>
>> >>Hongfu thank you for your patch, I was in middle of working and
>>testing
>> >>the patch and went ahead to commit the fix.
>> >>
>> >>
>> >>- Rohit
>> >>
>> >>
>> >>On February 13th, 2013, 5:07 a.m., Hongtu Zang wrote:
>> >>Review request for cloudstack, mice xia, anthony xu, and
>>SrikanteswaraRao
>> >>Talluri.
>> >>By Hongtu Zang.
>> >>
>> >>Updated Feb. 13, 2013, 5:07 a.m.
>> >>
>> >>Description
>> >>
>> >>In TemplateManagerImpl.java, function getAdapter(),
>> >>TemplateAdapterType.Hypervisor.getName() returns "HyervisorAdapter",
>> >>while it should returns "HyervisorTemplateAdapter". So, in
>> >>AdapterBase.java function getAdapterByName() returns null.
>> >>
>> >>
>> >>Testing
>> >>
>> >>register a template and start a vm.
>> >>
>> >>success.
>> >>
>> >>Bugs: CLOUDSTACK-1237, CLOUDSTACK-1240
>> >>Diffs
>> >>
>> >> *   server/src/com/cloud/template/TemplateAdapter.java (19cfef0)
>> >>
>> >>View Diff<https://reviews.apache.org/r/9420/diff/>
>> >>
>> >>
>> >
>> 


Mime
View raw message