Allon Mureinik has posted comments on this change.
Change subject: core,webadmin: Failure to reconnect authenticated iSCSI LUNs
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(5 inline comments)
....................................................
File
backend/manager/dbscripts/upgrade/03_01_1440_change_ssc_chap_password_to_text.sql
Line 1: -- change password in table storage_server_connections from varchar(50)
to text
Line 2: -- to account for password encryption
Line 3:
Line 4: select
fn_db_change_column_type('storage_server_connections','password','VARCHAR','text');
This would be far better, IMHO.
Also, can you explain this change? text is just a really long varchar. Why is
this required in order to encrypt passwords?
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacadeUtils.java
Line 24:
Line 25: public static Object asSingleResult(List<?> list) {
Line 26: return list.isEmpty() ? null : list.get(0);
Line 27: }
Line 28:
Aren't there unit test for this written for StorageConnectionDAO that should
also be moved around?
Line 29: private static Log log = LogFactory.getLog(DbFacadeUtils.class);
Line 30:
Line 31: public static String encryptPassword(String password) {
Line 32: if (StringUtils.isEmpty(password)) {
Line 25: public static Object asSingleResult(List<?> list) {
Line 26: return list.isEmpty() ? null : list.get(0);
Line 27: }
Line 28:
Line 29: private static Log log = LogFactory.getLog(DbFacadeUtils.class);
The log should be final, and placed either at the begining or the end of the
class, not in the middle of it.
Line 30:
Line 31: public static String encryptPassword(String password) {
Line 32: if (StringUtils.isEmpty(password)) {
Line 33: return password;
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java
Line 1620: storageDomain.setstorage_name((String)
model.getName().getEntity());
Line 1621:
Line 1622: if ((Boolean) sanModel.getUseUserAuth().getEntity()) {
Line 1623: storage_server_connections connection = new
storage_server_connections();
Line 1624: connection.setuser_name((Boolean)
sanModel.getUseUserAuth().getEntity() ? (String)
sanModel.getUserName().getEntity() : ""); //$NON-NLS-1$
this block is called inside " if ((Boolean)
sanModel.getUseUserAuth().getEntity())" - you know it evaluates to true, so the
"?" operator is redundant.
Line 1625: connection.setpassword((Boolean)
sanModel.getUseUserAuth().getEntity() ? (String)
sanModel.getPassword().getEntity() : ""); //$NON-NLS-1$
Line 1626: storageDomain.setConnection(connection);
Line 1627: }
Line 1628:
Line 1621:
Line 1622: if ((Boolean) sanModel.getUseUserAuth().getEntity()) {
Line 1623: storage_server_connections connection = new
storage_server_connections();
Line 1624: connection.setuser_name((Boolean)
sanModel.getUseUserAuth().getEntity() ? (String)
sanModel.getUserName().getEntity() : ""); //$NON-NLS-1$
Line 1625: connection.setpassword((Boolean)
sanModel.getUseUserAuth().getEntity() ? (String)
sanModel.getPassword().getEntity() : ""); //$NON-NLS-1$
here too.
Line 1626: storageDomain.setConnection(connection);
Line 1627: }
Line 1628:
Line 1629: if (isNew)
--
To view, visit http://gerrit.ovirt.org/8344
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I15b4cba7418d9d818fb2fd69c708fdeb20942f9c
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Greg Padgett <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
