On May 30, 2017 11:28, "Juan Hernández" <jhernand@redhat.com> wrote:
On 05/30/2017 09:38 AM, Tomas Jelinek wrote:
>
>
> On Tue, May 30, 2017 at 9:16 AM, Juan Hernández <jhernand@redhat.com
> <mailto:jhernand@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@redhat.com <mailto:mskrivan@redhat.com>
>     > <mailto:mskrivan@redhat.com <mailto:mskrivan@redhat.com>>> wrote:
>     >
>     >     > On 29 May 2017, at 11:44, Juan Hernández <jhernand@redhat.com <mailto:jhernand@redhat.com>
>     <mailto:jhernand@redhat.com <mailto:jhernand@redhat.com>>> wrote:
>     >     >
>     >     >> On 05/29/2017 11:27 AM, Michal Skrivanek wrote:
>     >     >>
>     >     >>> On 29 May 2017, at 10:39, Juan Hernández <jhernand@redhat.com <mailto:jhernand@redhat.com>
>     >     <mailto:jhernand@redhat.com <mailto:jhernand@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>
>     >     <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>
>     >
>      <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>
>     >
>      <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 makes sense to me. I have opened the following bug to track that:

  [RFE] Add properties to events
  https://bugzilla.redhat.com/1456711

Note that for the particular case of the VM name, which is what ManageIQ
is trying to do in that caode, the current best way is to use the <vm
.../> link that is part of the event. I have opened the following
ManageIQ issue to track it:

  Avoid parsing the descriptions of events
  https://github.com/ManageIQ/manageiq-providers-ovirt/issues/45

I think it was introduced while working on targeted refresh, but if we have an alternative then of course it would be better. 


>
>     >
>     >     >
>     >     > 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@ovirt.org <mailto:Devel@ovirt.org>
>     <mailto:Devel@ovirt.org <mailto:Devel@ovirt.org>>
>     >     >>> http://lists.ovirt.org/mailman/listinfo/devel
>     <http://lists.ovirt.org/mailman/listinfo/devel>
>     >     <http://lists.ovirt.org/mailman/listinfo/devel
>     <http://lists.ovirt.org/mailman/listinfo/devel>>
>     >     >
>     >     _______________________________________________
>     >     Devel mailing list
>     >     Devel@ovirt.org <mailto:Devel@ovirt.org>
>     <mailto:Devel@ovirt.org <mailto:Devel@ovirt.org>>
>     >     http://lists.ovirt.org/mailman/listinfo/devel
>     <http://lists.ovirt.org/mailman/listinfo/devel>
>     >     <http://lists.ovirt.org/mailman/listinfo/devel
>     <http://lists.ovirt.org/mailman/listinfo/devel>>
>     >
>     >
>
>

_______________________________________________
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel