Juan Hernandez has posted comments on this change. Change subject: restapi: Introduction of new TimeZone Complex Type ......................................................................
Patch Set 1: (5 comments) https://gerrit.ovirt.org/#/c/42150/1/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd File backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd: Line 3305: Line 3306: <xs:complexType name="TimeZone"> Line 3307: <xs:sequence> Line 3308: <xs:element name="name" type="xs:string" minOccurs="1" maxOccurs="1"/> Line 3309: <xs:element name="utc_offset" type="xs:int" minOccurs="1" maxOccurs="1"/> I think that xs:string is better for the offset, and it shold be represented as usually displayed: <utc_offset>+03:00</utc_offset> Line 3310: </xs:sequence> Line 3311: </xs:complexType> Line 3312: Line 3313: <xs:complexType name="GuestInfo"> https://gerrit.ovirt.org/#/c/42150/1/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmBaseMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmBaseMapper.java: Line 130: DisplayDisconnectAction action = DisplayDisconnectAction.fromValue(model.getDisplay().getDisconnectAction()); Line 131: entity.setConsoleDisconnectAction(map(action, null)); Line 132: } Line 133: } Line 134: if (model.isSetTimezone()) { Add a comment before this line explaining that is old, deprecated in favour of TimeZone and that will be removed in 4.x. Line 135: String timezone = model.getTimezone(); Line 136: if (timezone.isEmpty()) { Line 137: timezone = null; // normalize default timezone representation Line 138: } Line 136: if (timezone.isEmpty()) { Line 137: timezone = null; // normalize default timezone representation Line 138: } Line 139: entity.setTimeZone(timezone); Line 140: } Then new "time_zone" should also be supported here: TimeZone timeZone = model.getTimeZone(); if (timeZone != null) { if (timeZone.isSetName()) { // Same that we do for the current "timezone" } } Line 141: if (model.isSetOrigin()) { Line 142: entity.setOrigin(VmMapper.map(model.getOrigin(), (OriginType) null)); Line 143: } Line 144: if (model.isSetStateless()) { Line 223: model.getTimeZone().setUtcOffset(TimeZoneType.WINDOWS_TIMEZONE.getStandardOffset(entity.getTimeZone())); Line 224: } else { Line 225: model.getTimeZone().setUtcOffset(TimeZoneType.GENERAL_TIMEZONE.getStandardOffset(entity.getTimeZone())); Line 226: } Line 227: model.getTimeZone().setName(entity.getTimeZone()); The RESTAPI shouldn't do this kind of business logic. If the backend isn't currently capable of providing the offset then it shouldn't be populated. I mean, the RESTAPI can perfectly return this: <time_zone> <name>Whatver/Whatever</name> </time_zone> Line 228: model.setTimezone(entity.getTimeZone()); Line 229: Line 230: if (entity.getCreationDate() != null) { Line 231: model.setCreationTime(DateMapper.map(entity.getCreationDate(), null)); Line 224: } else { Line 225: model.getTimeZone().setUtcOffset(TimeZoneType.GENERAL_TIMEZONE.getStandardOffset(entity.getTimeZone())); Line 226: } Line 227: model.getTimeZone().setName(entity.getTimeZone()); Line 228: model.setTimezone(entity.getTimeZone()); Add a comment before this line explaining that this is old, deprecated and will be removed in 4.x. Line 229: Line 230: if (entity.getCreationDate() != null) { Line 231: model.setCreationTime(DateMapper.map(entity.getCreationDate(), null)); Line 232: } -- To view, visit https://gerrit.ovirt.org/42150 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5ac16620c3de15597e284f8674cbf2984d2e646b Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
