Alona Kaplan has posted comments on this change. Change subject: webadmin: Add MAC pools in DC dialog ......................................................................
Patch Set 12: (3 comments) http://gerrit.ovirt.org/#/c/27792/12/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterListModel.java: Line 296: model.setTitle(constants.editDataCenterTitle()); Line 297: model.setHelpTag(HelpTag.edit_data_center); Line 298: model.setHashName("edit_data_center"); //$NON-NLS-1$ Line 299: model.getName().setEntity(dataCenter.getName()); Line 300: model.initSelectedMacPool(); > How can you be sure that an asynchronous call hasn't returned yet? Cause we have just one thread in GWT so even if a response was returned from the backend it is pushed to the end of the event queue. This call (model.initSelectedMacPool()) is the same 'task' in the queue as the query registration. So for sure. the response of the async query we be called after the code here. Line 301: Line 302: if (getSystemTreeSelectedItem() != null Line 303: && getSystemTreeSelectedItem().getType() == SystemTreeItemType.DataCenter) Line 304: { http://gerrit.ovirt.org/#/c/27792/12/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterModel.java: Line 324: } Line 325: } Line 326: } Line 327: Line 328: public void initSelectedMacPool() { > It's definitely not necessary. I also think it's bad practice to use setEnt Where is setSlectedItem() used in this class? The method here has to be called when allMacPools != null && dc != null. Since dc = getEntity() and 'entity' is not initialized via the ctor it sounds reasonable to call initSelectedMacPool() on setEntity() and not count on that the 'entity' is already initialized when mac pool query is returned. I don't see any disadvantages in this approach. You can also move the call to initSelectedMacPool() from 'onSuccess' of the mac pool query to items changed event of getMacPoolListModel(). Doing this you will make sure initSelectedMacPool() is called whenever it has to be called. And also you can change its modifier to private since you want need to control calls to this method outside the class. Line 329: Collection<MacPool> allMacPools = getMacPoolListModel().getItems(); Line 330: StoragePool dc = getEntity(); Line 331: if (allMacPools != null && dc != null) { Line 332: Guid macPoolId = dc.getMacPoolId(); Line 332: macPoolId Please check if macPoolId is null. If it is, don't enter the loop. -- To view, visit http://gerrit.ovirt.org/27792 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I338a5647ded866c6b7b07542e9a95ea3dc3a2594 Gerrit-PatchSet: 12 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Martin Mucha <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
