+1 or replace spring with lightweight Guice
On Thu, Feb 14, 2013 at 10:39 PM, Alex Huang <[email protected]> wrote: > > >> -----Original Message----- >> From: Chip Childers [mailto:[email protected]] >> Sent: Thursday, February 14, 2013 6:57 AM >> To: [email protected] >> 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 <[email protected]> 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" <[email protected]> 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: [email protected] [mailto:[email protected]] 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 >> > >><[email protected]<mailto:[email protected]>> 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/> >> > >> >> > >> >> > > >> >
