> On July 17, 2014, 4:55 p.m., Nitin Mehta wrote: > > server/src/com/cloud/vm/UserVmManagerImpl.java, line 960 > > <https://reviews.apache.org/r/23617/diff/1/?file=634289#file634289line960> > > > > Why is it made create=true when it is not a BaseAyncCreate cmd ? > > create=true should be added only when its invoked through the create() > > method of a BaseAyncCreate cmd > > Damodar Reddy Talakanti wrote: > We can not extend AddNicToVMCmd by BaseAsyncCreate cmd as it is doing > many checks before creating the entry NIC. As it is creating entity did made > create=true there. > > Nitin Mehta wrote: > Many checks is not a problem as long as they are DB checks. Do note we > make a command async generally because it is contacting the resource and can > take time. If it is just DB checks I would think we can make the command > BaseAsyncCreate. Also best that you review with Kishan if create=true will > generate all the events (scheduled, started, completed) an async command > generates > > Damodar Reddy Talakanti wrote: > I already verified, it is generating all the above events > (scheduled,started and created in the DB). It is not only issue of DB checks > it is creating the entity some where deep in the flow. To bring that up we > need to pull all that business logic into API create method which I though > not a good idea. So just made it create=true.
I understand but I guess we should do that. Giving a half baked solution will be a problem in the future. Dont you agree ? - Nitin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23617/#review48016 ----------------------------------------------------------- On July 17, 2014, 4:43 p.m., Damodar Reddy Talakanti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23617/ > ----------------------------------------------------------- > > (Updated July 17, 2014, 4:43 p.m.) > > > Review request for cloudstack and Nitin Mehta. > > > Repository: cloudstack-git > > > Description > ------- > > Putting the create event into event bus for addNicToVirutalMachine command > explicitly as we can not use BaseAsyncCreateCommand.We can not extend this > with BaseAsyncCreateCmd due to the checks we do before creating the nic. > > > Diffs > ----- > > api/src/com/cloud/event/EventTypes.java 71bfdb6 > server/src/com/cloud/vm/UserVmManagerImpl.java d0bc186 > > Diff: https://reviews.apache.org/r/23617/diff/ > > > Testing > ------- > > Tested against the following setup: > > 1. XenServer 6.2 > 2. Master Branch > > > Thanks, > > Damodar Reddy Talakanti > >
