cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Huang <Alex.Hu...@citrix.com>
Subject RE: Duplicate BareMetalTemplateAdapter classes
Date Thu, 14 Feb 2013 17:18:23 GMT
lol

> -----Original Message-----
> From: rohityadav89@gmail.com [mailto:rohityadav89@gmail.com] On Behalf
> Of Rohit Yadav
> Sent: Thursday, February 14, 2013 9:16 AM
> To: cloudstack-dev@incubator.apache.org
> Cc: Min Chen; Kelven Yang
> Subject: Re: Duplicate BareMetalTemplateAdapter classes
> 
> +1 or replace spring with lightweight Guice
> 
> On Thu, Feb 14, 2013 at 10:39 PM, Alex Huang <Alex.Huang@citrix.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Chip Childers [mailto:chip.childers@sungard.com]
> >> Sent: Thursday, February 14, 2013 6:57 AM
> >> To: cloudstack-dev@incubator.apache.org
> >> Cc: Min Chen
> >> Subject: Re: Duplicate BareMetalTemplateAdapter classes
> >>
> >> 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).
> >
> > +1 autoscanning has to be removed in 4.1.
> >
> > Kelven,
> >
> > I created https://issues.apache.org/jira/browse/CLOUDSTACK-1276 for this
> and assigned it to you.
> >
> > --Alex
> >>
> >> >
> >> > 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=commi
> >> > > 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
> /Ba
> >> reMetalT
> >> > >>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