Re: Review Request 23617: Add Nic UUID to the context so that we can read the same in event bus after create a nic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23617/ --- (Updated July 22, 2014, 9:15 a.m.) Review request for cloudstack and Nitin Mehta. Changes --- Incorporated changes suggested by Nitin. Adding Kishan to the reviewers list 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 (updated) - api/src/com/cloud/event/EventTypes.java 411f620 server/src/com/cloud/vm/UserVmManagerImpl.java 90f37ef 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
Re: Review Request 23617: Add Nic UUID to the context so that we can read the same in event bus after create a nic
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. Nitin Mehta wrote: I understand but I guess we should do that. Giving a half baked solution will be a problem in the future. Dont you agree ? Ok Agreed, will do the changes - Damodar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23617/#review48016 --- On July 22, 2014, 9:15 a.m., Damodar Reddy Talakanti wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23617/ --- (Updated July 22, 2014, 9:15 a.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 411f620 server/src/com/cloud/vm/UserVmManagerImpl.java 90f37ef 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
Re: Review Request 23617: Add Nic UUID to the context so that we can read the same in event bus after create a nic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23617/ --- (Updated July 22, 2014, 9:16 a.m.) Review request for cloudstack, Kishan Kavala 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 411f620 server/src/com/cloud/vm/UserVmManagerImpl.java 90f37ef 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
Re: Review Request 23617: Add Nic UUID to the context so that we can read the same in event bus after create a nic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23617/ --- (Updated July 22, 2014, 12:35 p.m.) Review request for cloudstack, Kishan Kavala 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 (updated) - api/src/com/cloud/event/EventTypes.java 411f620 server/src/com/cloud/vm/UserVmManagerImpl.java 90f37ef 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
Re: Review Request 23617: Add Nic UUID to the context so that we can read the same in event bus after create a nic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23617/#review48472 --- Ship it! - Kishan Kavala On July 22, 2014, 6:05 p.m., Damodar Reddy Talakanti wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23617/ --- (Updated July 22, 2014, 6:05 p.m.) Review request for cloudstack, Kishan Kavala 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 411f620 server/src/com/cloud/vm/UserVmManagerImpl.java 90f37ef 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
Re: Review Request 23617: Add Nic UUID to the context so that we can read the same in event bus after create a nic
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
Re: Review Request 23617: Add Nic UUID to the context so that we can read the same in event bus after create a nic
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 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. - Damodar Reddy --- 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
Re: Review Request 23617: Add Nic UUID to the context so that we can read the same in event bus after create a nic
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 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. - Damodar Reddy --- 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
Re: Review Request 23617: Add Nic UUID to the context so that we can read the same in event bus after create a nic
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. 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 - 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
Re: Review Request 23617: Add Nic UUID to the context so that we can read the same in event bus after create a nic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23617/#review48016 --- server/src/com/cloud/vm/UserVmManagerImpl.java https://reviews.apache.org/r/23617/#comment84267 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 - Nitin Mehta 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
