Ori Liel has posted comments on this change.
Change subject: restapi: #838270 - Support View Of System Configuration
......................................................................
Patch Set 11: (5 inline comments)
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendConfigurationItemsResource.java
Line 28: new
GetConfigurationValuesQueryParameters(false), null);
Line 29: return addActionLinks(mapCollection(itemsMap));
Line 30: }
Line 31:
Line 32: private ConfigurationItems
mapCollection(Map<KeyValuePairCompat<ConfigurationValues, String>, Object>
itemsMap) {
Most mapping code is already in a mapper. Here I only loop over entries in the
map and for each one, activate that mapper (Entry-->ConfigurationItem)
I could add another mapping method in the mapper:
Map<KeyValuePairCompat<ConfigurationValues, String>, Object> -->
ConfigurationItems
and move these lines to there, but it's such a strange signature that it
doesn't 't feel right.
Line 33: ConfigurationItems items = new ConfigurationItems();
Line 34: for (Entry<KeyValuePairCompat<ConfigurationValues, String>,
Object> entry : itemsMap.entrySet()) {
Line 35: ConfigurationItem item = getMapper(Entry.class,
ConfigurationItem.class).map(entry, null);
Line 36: // item may be null because of missing mapping, and we
don't want to fail the operation for that,
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendHostHookResource.java
Line 21
Line 22
Line 23
Line 24
Line 25
Correct. I scanned the code for places that generate IDs using MD5 (to make
them use the new MD5Tools.java, which I have added in this patch). I ran into
this bug and fixed it along the way. I'll move it to another patch.
....................................................
File
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VersionMapper.java
Line 24: if (model == null) {
Line 25: model = new Version();
Line 26: }
Line 27: model.setMajor(entity.getMajor());
Line 28: model.setMinor(entity.getMinor());
We don't support them in API. If they are mapped - i.e have a value - the value
will be displayed to the user. I don't think we want that.
Line 29: return model;
Line 30: }
Line 31:
Line 32: /**
Line 42: model.setNonSpecific(true);
Line 43: } else {
Line 44: model = VersionUtils.parseVersion(entity);
Line 45: model.setBuild(null); // API currently doesn't support
build
Line 46: model.setRevision(null); // API currently doesn't support
revision
Again, if we map them, we'll display them. I don't want to show the user
something which is not fully conceptualized/supported in the API layer, and
then have to deal with backward compatibility.
Line 47: }
Line 48: return model;
Line 49: }
Line 50:
Line 51: @Mapping(from = Version.class, to =
org.ovirt.engine.core.compat.Version.class)
Line 52: public static org.ovirt.engine.core.compat.Version map(Version
model, org.ovirt.engine.core.compat.Version entity) {
Line 53: // Note: currently API layer only supports major & minor
properties of Version, but this may change in the
Line 54: // future (to include build and revision as well).
Line 55: return new
org.ovirt.engine.core.compat.Version(model.getMajor(), model.getMinor());
Same reason, not supported, although here it's slightly different since it
doesn't involve the element of display (it's API-->Backend direction). Still,
if you'll agree with me about the above two methods, then for uniformity, the
partial mapping is justified here as well.
Line 56: }
Line 57:
--
To view, visit http://gerrit.ovirt.org/11468
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d5cf3a09cfbf4bfb7586ba296e13f71386c396f
Gerrit-PatchSet: 11
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ori Liel <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Ori Liel <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
