Juan Hernandez has posted comments on this change. Change subject: packaging: Application Mode option in installer ......................................................................
Patch Set 4: (6 inline comments) .................................................... File backend/manager/dbscripts/common_sp.sql Line 540: RETURN; Line 541: END; $procedure$ Line 542: LANGUAGE plpgsql; Line 543: Line 544: -- Updates service types(gluster and virt) in vds_groups table There is a inst_sq.sql that contains some stored procedures used by the installer, used to set the default storage domain time and to create the ISO domain. You may want to move this procedure to that file and name it like the other procedures there: inst_whatever. Line 545: create or replace FUNCTION fn_db_update_service_type(v_cluster_id varchar(40), v_virt_service boolean, Line 546: v_gluster_service boolean) Line 547: returns void Line 548: AS $procedure$ Line 541: END; $procedure$ Line 542: LANGUAGE plpgsql; Line 543: Line 544: -- Updates service types(gluster and virt) in vds_groups table Line 545: create or replace FUNCTION fn_db_update_service_type(v_cluster_id varchar(40), v_virt_service boolean, As the v_cluster_id is an UUID you can use here it as the type: v_cluster_id uuid Line 546: v_gluster_service boolean) Line 547: returns void Line 548: AS $procedure$ Line 549: begin Line 547: returns void Line 548: AS $procedure$ Line 549: begin Line 550: update vds_groups set virt_service = v_virt_service, gluster_service = v_gluster_service Line 551: where vds_group_id = cast(v_cluster_id as uuid); If you change the type of the paramter to UUID then this cast is not needed. Line 552: END; $procedure$ .................................................... File packaging/fedora/setup/basedefs.py Line 175: CONST_CONFIG_EXTRA_IPTABLES_RULES="EXTRA_IPTABLES_RULES" Line 176: CONST_DEFAULT_CLUSTER_ID="99408929-82CF-4DC7-A532-9D998063FA95" Line 177: CONST_DEFAULT_APPLICATION_MODE="both" Line 178: Line 179: SHOW_APPLICATION_MODE_IN_SETUP=True I would suggest to add a comment here explaining what this constant means. Line 180: Line 181: # This is needed for avoiding error in create_ca when supporting max cn length of 64. Line 182: # please DONT increase this size, any value over 55 will fail the setup. Line 183: # the truncated host-fqdn is concatenated with a random string to create a unique CN value. .................................................... File packaging/fedora/setup/engine-setup.py Line 979: output_messages.ERR_DB_SET_SERVICE_TYPE) Line 980: Line 981: # No change with respect to default application mode, No DB update required Line 982: if controller.CONF["APPLICATION_MODE"].upper() == "BOTH": Line 983: return I don't understand this very well. The comment says that no db update is required, but this code runs after the execition of the fn_db_update_service_type function. Shouldn't this check go at the very beginning of the function? Line 984: Line 985: applicationMode = controller.CONF["APPLICATION_MODE_ENUM"].parse(str.upper(controller.CONF["APPLICATION_MODE"])) Line 986: Line 987: sqlQuery = "select fn_db_update_config_value('ApplicationMode', '%s', 'general')" % applicationMode .................................................... File packaging/fedora/setup/output_messages.py Line 141: INFO_CONF_PARAMS_DB_PASSWD_USAGE="Password for the local database administrator" Line 142: INFO_CONF_PARAMS_DB_PASSWD_PROMPT="Enter a password for a local %s DB admin user (%s)" % (basedefs.APP_NAME, basedefs.DB_USER) Line 143: INFO_CONF_PARAMS_PASSWD_CONFIRM_PROMPT="Confirm password" Line 144: INFO_CONF_PARAMS_APPLICATION_MODE_USAGE="Application Mode" Line 145: INFO_CONF_PARAMS_APPLICATION_MODE_PROMPT="Application Mode" We probably need a longer explanation so that the user will understand what this question means. Line 146: Line 147: #Remote DB interaction Line 148: INFO_CONF_PARAMS_REMOTE_DB_USAGE="Select local or remote DB server" Line 149: INFO_CONF_PARAMS_REMOTE_DB_PROMPT="Enter DB type for installation" -- To view, visit http://gerrit.ovirt.org/9991 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieba515d2baf3559bf27185c7c6432160551ef172 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Aravinda VK <avish...@redhat.com> Gerrit-Reviewer: Aravinda VK <avish...@redhat.com> Gerrit-Reviewer: Bala.FA <barum...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Moran Goldboim <mgold...@redhat.com> Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com> Gerrit-Reviewer: Ohad Basan <oba...@redhat.com> Gerrit-Reviewer: Ronen Angluster <rangl...@redhat.com> Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches