Alissa Bonas has posted comments on this change.
Change subject: engine: StorageConnections ops won't assume VDS
......................................................................
Patch Set 1: (2 inline comments)
1. are storagepool/host id passed properly in delete/update usecase in
StorageListModel?
2. what about REST? currently we do require host to be passed (and validate it)
in both storagedomains resource, and in the future (not merged yet) connections
resource.
So the restriction/requirement should be removed, AND documented in REST
documentation - since it affects the actual behavior of the action
(with/without connect/disconnect attempts)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java
Line 48: success = false;
Line 49: }
Line 50: }
Line 51:
Line 52: setSucceeded(success);
no need to set it twice. it can be just set in the last line of the method.
Line 53:
Line 54: if (success) {
Line 55: connection.setid(Guid.newGuid().toString());
Line 56: saveConnection(connection);
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java
Line 26: import org.ovirt.engine.core.dao.StorageServerConnectionDAO;
Line 27: import org.ovirt.engine.core.utils.MockEJBStrategyRule;
Line 28:
Line 29: @RunWith(MockitoJUnitRunner.class)
Line 30: public class AddStorageServerConnectionCommandTest {
I would add a test when vdsid is null, and not Guid.empty - just to be on the
same side the command works fine
Line 31: @ClassRule
Line 32: public static MockEJBStrategyRule ejbRule = new
MockEJBStrategyRule();
Line 33:
Line 34: private
AddStorageServerConnectionCommand<StorageServerConnectionParametersBase>
command = null;
--
To view, visit http://gerrit.ovirt.org/17288
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I628bf2a63f64d3acaa3520ad144e1eef744f1204
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
