Roy Golan has posted comments on this change.
Change subject: engine: Cache spice support for os/compatibility version
......................................................................
Patch Set 2:
(4 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OsRepositoryQuery.java
Line 35: break;
Line 36: case GetMinimumOsRam:
Line 37:
setReturnValue(osRepository.getMinimumRam(getParameters().getOsId(),
getParameters().getVersion()));
Line 38: break;
Line 39: case HasSpiceSupportMatrix:
please rename to GetSpiceSupportMatrix instead of HasSpice...
Line 40: setReturnValue(osRepository.spiceSupportMatrix());
Line 41: break;
Line 42: case GetNetworkDevices:
Line 43:
setReturnValue(osRepository.getNetworkDevices(getParameters().getOsId(),
getParameters().getVersion()));
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/osinfo/OsRepository.java
Line 68: /**
Line 69: * @return map (osId -> compatibility version -> true/false) that
indicates SPICE support for all OSs and
Line 70: * compatibility versions
Line 71: */
Line 72: public Map<Integer, Map<Version, Boolean>> spiceSupportMatrix();
please rename to getSpiceSupportMatrix
BTW is GWT happy with Map and not HashMap signature?
Line 73:
Line 74: /**
Line 75: * this is Windows OSs specific path to the sysprep file
Line 76: * @param osId
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/osinfo/OsRepositoryImpl.java
Line 151:
Line 152: for (Integer osId : getOsIds()) {
Line 153: spiceSupportMatrix.put(osId, new HashMap<Version,
Boolean>());
Line 154:
Line 155: Set<Version> versions = new HashSet<Version>(Version.ALL);
no need to allocate this set over and over again.
Line 156: versions.add(null);
Line 157:
Line 158: for (Version ver : versions) {
Line 159: boolean spiceSupport =
getBoolean(getValueByVersion(idToUnameLookup.get(osId), "spiceSupport", ver),
false);
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java
Line 3083:
Line 3084: return osNames.get(osId);
Line 3085: }
Line 3086:
Line 3087: public static Boolean hasSpiceSupport(int osId, Version version)
{
why the NPE is there in first place?
the int in the signature is saying NO NULLS here and helps you understand where
is the (bad) code that sends null and also leave you with no doubt.
with null friendly method you won't get a real answer about the spice support
which will lead to bugs.
or maybe I'm missing some UI practice here?
Line 3088: return spiceSupportMatrix.get(osId).get(version);
Line 3089: }
Line 3090:
Line 3091: private static void initHasSpiceSupport() {
--
To view, visit http://gerrit.ovirt.org/19137
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib97d68ccd0d30fd07e8d1a7d1322f731aef220f7
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Frank Kobzik <fkob...@redhat.com>
Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
