[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #3712: Replace builtin CentOS 5.x with 8.0 templates

[GitBox <gi...@apache.org>]

GabrielBrascher commented on a change in pull request #3712: Replace builtin CentOS 5.x with 8.0 templates
URL: https://github.com/apache/cloudstack/pull/3712#discussion_r350338687
 
 

 ##########
 File path: engine/schema/src/main/resources/META-INF/db/schema-41300to41400-cleanup.sql
 ##########
 @@ -19,3 +19,6 @@
 -- Schema upgrade cleanup from 4.13.0.0 to 4.14.0.0
 --;
 
+-- Delete an ancient template added in 2.2, which was set as removed in 4.5. For the sake of cleanup and easier overview of builtin templates in DB.
+-- This DOES assume one is not running any VMs from this template!
+DELETE FROM `cloud`.`vm_template` WHERE `id`=2 AND `unique_name`="centos53-x86_64" AND `name`="CentOS 5.3(64-bit) no GUI (XenServer)";
 
 Review comment:
   Considering:
   > "This DOES assume one is not running any VMs from this template!"
   
   Does it make sense to add an extra verification that ensures the deletion only if there is no VMs on with that template ID (therefore deleting only if does not exists VMs with _removed NULL_)?
   
   For instance:
   ```
   DELETE FROM `cloud`.`vm_template` WHERE `id`=2 AND `unique_name`="centos53-x86_64" AND `name`="CentOS 5.3(64-bit) no GUI (XenServer)" AND NOT EXISTS (SELECT 1 FROM `cloud`.`vm_instance` WHERE vm_template_id=2 AND removed IS NULL);
   ```
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services