[ovirt-devel] Making event types backwards compatible?

Tomas Jelinek tjelinek at redhat.com
Tue May 30 07:38:16 UTC 2017


On Tue, May 30, 2017 at 9:16 AM, Juan Hernández <jhernand at redhat.com> wrote:

> On 05/30/2017 08:55 AM, Tomas Jelinek wrote:
> >
> >
> > On Tue, May 30, 2017 at 7:20 AM, Michal Skrivanek <mskrivan at redhat.com
> > <mailto:mskrivan at redhat.com>> wrote:
> >
> >     > On 29 May 2017, at 11:44, Juan Hernández <jhernand at redhat.com
> <mailto:jhernand at redhat.com>> wrote:
> >     >
> >     >> On 05/29/2017 11:27 AM, Michal Skrivanek wrote:
> >     >>
> >     >>> On 29 May 2017, at 10:39, Juan Hernández <jhernand at redhat.com
> >     <mailto:jhernand at redhat.com>> 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 at 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
> >     >>> Devel at ovirt.org <mailto:Devel at ovirt.org>
> >     >>> http://lists.ovirt.org/mailman/listinfo/devel
> >     <http://lists.ovirt.org/mailman/listinfo/devel>
> >     >
> >     _______________________________________________
> >     Devel mailing list
> >     Devel at ovirt.org <mailto:Devel at ovirt.org>
> >     http://lists.ovirt.org/mailman/listinfo/devel
> >     <http://lists.ovirt.org/mailman/listinfo/devel>
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/devel/attachments/20170530/dd4492dc/attachment.html>


More information about the Devel mailing list