On Tue, May 30, 2017 at 9:16 AM, Juan Hernández <[email protected]> wrote:
> On 05/30/2017 08:55 AM, Tomas Jelinek wrote: > > > > > > On Tue, May 30, 2017 at 7:20 AM, Michal Skrivanek <[email protected] > > <mailto:[email protected]>> wrote: > > > > > On 29 May 2017, at 11:44, Juan Hernández <[email protected] > <mailto:[email protected]>> wrote: > > > > > >> On 05/29/2017 11:27 AM, Michal Skrivanek wrote: > > >> > > >>> On 29 May 2017, at 10:39, Juan Hernández <[email protected] > > <mailto:[email protected]>> wrote: > > >>> > > >>> Hello, > > >>> > > >>> It has been recently requested that the API provides event types: > > >>> > > >>> [RFE] Expose event types to API > > >>> https://bugzilla.redhat.com/1453170 > > <https://bugzilla.redhat.com/1453170> > > >>> > > >>> Currently the API provides the event code and description, for > > example: > > >>> > > >>> <event href="/ovirt-engine/api/events/8021" id="8021"> > > >>> <code>19</code> > > >>> <description>Host myhost failed to recover.</description > > >>> ... > > >>> </event> > > >>> > > >>> There is no documentation of what is the meaning of codes, > > except the > > >>> source code of the engine itself. This forces some applications > > to add > > >>> their own code to name mapping. For example, the 'ovirt' Ruby > > gem used > > >>> by older versions of ManageIQ to interact with oVirt contains > > the following: > > >>> > > >>> > > https://github.com/ManageIQ/ovirt/blob/v0.17.0/lib/ovirt/ > event.rb#L25-L485 > > <https://github.com/ManageIQ/ovirt/blob/v0.17.0/lib/ovirt/ > event.rb#L25-L485> > > >>> > > >>> We could avoid this by adding to the API a new event attribute > that > > >>> indicates the type: > > >>> > > >>> <event href="/ovirt-engine/api/events/8021" id="8021"> > > >>> <code>19</code> > > >>> <type>host_recover_failure</type> > > >>> <description>Host myhost failed to recover.</description> > > >>> ... > > >>> </event> > > >>> > > >>> Ideally this should be defined as an enum, so that it will be > > >>> represented as an enum in the SDKs. Alternatively it could just > > be an > > >>> string, and we could reuse the 'name' attribute: > > >>> > > >>> <event href="/ovirt-engine/api/events/8021" id="8021"> > > >>> <code>19</code> > > >>> <name>host_recover_failure</name> > > >>> <description>Host myhost failed to recover.</description> > > >>> ... > > >>> </event> > > >>> > > >>> However, the key point to making this useful would be to keep > > the types > > >>> (or names) backwards compatible, so that users of the API can > > rely on > > >>> their values and meanings. > > >>> > > >>> So this is my question to you: can we commit to keep the names > and > > >>> meanings of the backend event types backwards compatible? > > >> > > >> Do we even have to make it bw compatible? > > >> I guess it depends on the actual usage of those names… > > >> The ovirt ruby gem itself doesn’t do much with it > > > > > > We need to make keep it backwards compatible or else tell users > "don't > > > rely on these values, as they may change without notice". > > > > > > The 'ovirt' gem doesn't do anything special, it just creates its > own > > > code to name mapping. But the users of the 'ovirt' gem (the > ManageIQ > > > oVirt provider) do rely on the name. For example: > > > > > > > > > https://github.com/ManageIQ/manageiq-providers-ovirt/blob/ > master/app/models/manageiq/providers/redhat/infra_ > manager/event_parser.rb#L80-L92 > > <https://github.com/ManageIQ/manageiq-providers-ovirt/blob/ > master/app/models/manageiq/providers/redhat/infra_ > manager/event_parser.rb#L80-L92> > > > > > > hmmm, while we are on topic, this pretty much looks like that manageiq > > does not only rely on the code but also on the actual value of it since > > it is parsing it: > > > > # sample message: "Interface nic1 (VirtIO) was added to VM v5. (User: > > admin@internal-authz)" message.split(/\s/)[7][0...-1] > > > > Is this something we commit to maintain? Or should we commit to maintain > it? > > > > That is a good point, that isn't very future proof. We should also find > a way to make less fragile. Any suggestion? > The only doable thing which comes to my mind is something like this: The msg is defined like this: USER_ADD_VM_POOL_WITH_VMS_FAILED=Failed to create VM Pool ${VmPoolName} (User: ${UserName}). e.g. the msg type and the variables. If we could expose in the api not only the substituted msg but also the variable/value binding, we could commit to keep the variable names backward compatible. So, something like: <event href="/ovirt-engine/api/events/8021" id="8021"> <code>19</code> <type>USER_ADD_VM_POOL_WITH_VMS_FAILED</type> <description>the substituted msg.</description> <parameters> <parameter> <key>VmPoolName</key> <value>The Pool Name<value> </parameter> ... </parameters> </event> Not really rock solid since the variables would still be defined in the AuditLogMessages.properties but still better and still easier to parse on the client side. > > > > > > > > That means that if we ever change the meaning of a code the > ManageIQ > > > provider, for example, will break. > > > > Right,then it indeed needs to stay stable. > > But how is maintaining the enum string different from the code? It is > > the same information, so if MIQ doesn't use the name directly then it > > doesn't really matter if it's a code or string. > > Perhaps deprecate the code and keep the name fixed? > > > > Thanks, > > michal > > > > > > > >>> > > >>> Regards, > > >>> Juan Hernandez > > >>> > > >>> > > >>> _______________________________________________ > > >>> Devel mailing list > > >>> [email protected] <mailto:[email protected]> > > >>> http://lists.ovirt.org/mailman/listinfo/devel > > <http://lists.ovirt.org/mailman/listinfo/devel> > > > > > _______________________________________________ > > Devel mailing list > > [email protected] <mailto:[email protected]> > > http://lists.ovirt.org/mailman/listinfo/devel > > <http://lists.ovirt.org/mailman/listinfo/devel> > > > > > >
_______________________________________________ Devel mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/devel
