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

Reply via email to