Michael Pasternak has posted comments on this change.
Change subject: restapi: #853739 - Actions Should Return Entities In Body
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(4 inline comments)
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendActionableResource.java
Line 84: } else {
Line 85: return actionSuccess(action, addLinks(newModel(id)));
Line 86: }
Line 87: } catch (Exception e) {
Line 88: e.printStackTrace();
do you need this?
Line 89: return handleError(e, action);
Line 90: }
Line 91: }
Line 92:
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java
Line 306:
Line 307: protected Object removeUnnecessaryInfo(Object entity) {
Line 308: String methodName;
Line 309: Object nullObj = null;
Line 310: for (Method m : entity.getClass().getMethods()) {
i'd create new instance of the same object by [1] (you can always do that as
all types
are serializeable and therefore have default constructor),
and copy only id & href variables to it instead of looping over all fields and
setting
NULLs, as this is more efficient and less error prone
[1] entity.getClass().newInstance()
Line 311: methodName = m.getName();
Line 312: if (methodName.startsWith("set") &&
Line 313: !methodName.equals("setId") &&
Line 314: !methodName.equals("setHref") &&
Line 311: methodName = m.getName();
Line 312: if (methodName.startsWith("set") &&
Line 313: !methodName.equals("setId") &&
Line 314: !methodName.equals("setHref") &&
Line 315: !methodName.equals("setLinks")) {
links is not needed, only id+href
Line 316: try {
Line 317: m.invoke(entity, nullObj);
Line 318: } catch (IllegalAccessException ex) {
Line 319: LOG.error("Entity Null value assignment failure
"+methodName, ex);
....................................................
File backend/manager/tools/src/main/shell/engine-notifier.sh
Line 222: -jar "${JBOSS_HOME}/jboss-modules.jar" \
Line 223: -dependencies org.ovirt.engine.core.tools \
Line 224: -class org.ovirt.engine.core.notifier.Notifier \
Line 225: "${CONF_FILE}" \
Line 226: 2>/dev/null
this change does not belong the patch
Line 227:
--
To view, visit http://gerrit.ovirt.org/9466
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic636b7205fa7b6fbb5d98efdbcd7976e9beb2187
Gerrit-PatchSet: 2
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: Ravi Nori <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
