Alon Bar-Lev has posted comments on this change.
Change subject: Using specific username and password to install and update host
......................................................................
Patch Set 1: (7 inline comments)
Let me understand... maybe I miss something... this is what I expect:
1. add user name to vds table.
2. when adding a host, we create an object of vds with the user.
3. we can use vds object to fetch the user name all over application, no need
to pass it all over.
4. password is non persistent, so we should send it as parameter all over.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RegisterVdsQuery.java
Line 274: getStrippedVdsUniqueId(),
Line 275: getParameters().getPort(),
Line 276: IsPending);
Line 277:
Line 278: UpdateVdsActionParameters p = new
UpdateVdsActionParameters(vdsByUniqueId.getStaticData(), "root", "", false);
why hardcode root?
Line 279:
p.setTransactionScopeOption(TransactionScopeOption.RequiresNew);
Line 280: VdcReturnValueBase rc =
Backend.getInstance().runInternalAction(VdcActionType.UpdateVds, p);
Line 281:
Line 282: if (rc == null || !rc.getSucceeded()) {
Line 322: getStrippedVdsUniqueId(),
Line 323: getParameters().getPort(),
Line 324: IsPending);
Line 325:
Line 326: AddVdsActionParameters p = new
AddVdsActionParameters(vds, "root", "");
same...
Line 327: p.setAddPending(IsPending);
Line 328:
Line 329: VdcReturnValueBase ret =
Backend.getInstance().runInternalAction(VdcActionType.AddVds, p);
Line 330:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsCommand.java
Line 87: && _oldVds.getStatus() !=
VDSStatus.InstallFailed) {
Line 88:
addCanDoActionMessage(VdcBllMessages.VDS_CANNOT_INSTALL_STATUS_ILLEGAL);
Line 89: } else if (getParameters().getInstallVds()
Line 90: &&
StringUtils.isEmpty(getParameters().getPassword())
Line 91: &&
StringUtils.isEmpty(getParameters().getUsername())
try to keep consistent... always username then password
Line 92: &&
getParameters().getVdsStaticData().getVdsType() == VDSType.VDS) {
Line 93:
addCanDoActionMessage(VdcBllMessages.VDS_CANNOT_INSTALL_EMPTY_PASSWORD);
Line 94: } else if (!getParameters().getInstallVds()
Line 95: && _oldVds.getPort() !=
getParameters().getVdsStaticData().getPort()) {
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVdsCommandTest.java
Line 74: @Before
Line 75: public void createParameters() {
Line 76: parameters = new AddVdsActionParameters();
Line 77: parameters.setPassword("secret");
Line 78: parameters.setUsername("root");
first user then password
Line 79: VDS newVds = makeTestVds(vdsId);
Line 80: parameters.setvds(newVds);
Line 81: }
Line 82:
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/InstallVdsParameters.java
Line 19: }
Line 20:
Line 21: public InstallVdsParameters(Guid vdsId, String username, String
password) {
Line 22: super();
Line 23: this.setVdsId(vdsId);
I would like us to retrieve the user from vds object if possible.
Line 24: setUsername(username);
Line 25: setPassword(password);
Line 26: }
Line 27:
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/UpdateVdsActionParameters.java
Line 10: _installVds = installVds;
Line 11: }
Line 12:
Line 13: public UpdateVdsActionParameters(VdsStatic vdsStatic, boolean
installVds) {
Line 14: super(vdsStatic);
same here... if we store user within vdsStatic we can retrieve it from there.
Line 15: _installVds = installVds;
Line 16: }
Line 17:
Line 18: private boolean _installVds;
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdsOperationActionParameters.java
Line 50: public String getUsername() {
Line 51: return _username;
Line 52: }
Line 53:
Line 54: public void setUsername(String value) {
I think the convention is to use username and not value, same for password.
Line 55: _username = value;
Line 56: }
Line 57:
Line 58: public VdsOperationActionParameters() {
--
To view, visit http://gerrit.ovirt.org/16006
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id39914e1286870373ad6f69360fa7b3ddfabd8df
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
