<div dir="ltr"><br><div><div><div><div><div class="gmail_extra"><div class="gmail_quote">On Tue, May 30, 2017 at 11:28 AM, Juan Hernández <span dir="ltr"><<a href="mailto:jhernand@redhat.com" target="_blank">jhernand@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-">On 05/30/2017 09:38 AM, Tomas Jelinek wrote:<br>
><br>
><br>
> On Tue, May 30, 2017 at 9:16 AM, Juan Hernández <<a href="mailto:jhernand@redhat.com">jhernand@redhat.com</a><br>
</span><span class="gmail-">> <mailto:<a href="mailto:jhernand@redhat.com">jhernand@redhat.com</a>>> wrote:<br>
><br>
> On 05/30/2017 08:55 AM, Tomas Jelinek wrote:<br>
> ><br>
> ><br>
> > On Tue, May 30, 2017 at 7:20 AM, Michal Skrivanek <<a href="mailto:mskrivan@redhat.com">mskrivan@redhat.com</a> <mailto:<a href="mailto:mskrivan@redhat.com">mskrivan@redhat.com</a>><br>
</span><span class="gmail-">> > <mailto:<a href="mailto:mskrivan@redhat.com">mskrivan@redhat.com</a> <mailto:<a href="mailto:mskrivan@redhat.com">mskrivan@redhat.com</a>>>> wrote:<br>
> ><br>
> > > On 29 May 2017, at 11:44, Juan Hernández <<a href="mailto:jhernand@redhat.com">jhernand@redhat.com</a> <mailto:<a href="mailto:jhernand@redhat.com">jhernand@redhat.com</a>><br>
</span><span class="gmail-">> <mailto:<a href="mailto:jhernand@redhat.com">jhernand@redhat.com</a> <mailto:<a href="mailto:jhernand@redhat.com">jhernand@redhat.com</a>>>> wrote:<br>
> > ><br>
> > >> On 05/29/2017 11:27 AM, Michal Skrivanek wrote:<br>
> > >><br>
> > >>> On 29 May 2017, at 10:39, Juan Hernández <<a href="mailto:jhernand@redhat.com">jhernand@redhat.com</a> <mailto:<a href="mailto:jhernand@redhat.com">jhernand@redhat.com</a>><br>
</span><div><div class="gmail-h5">> > <mailto:<a href="mailto:jhernand@redhat.com">jhernand@redhat.com</a> <mailto:<a href="mailto:jhernand@redhat.com">jhernand@redhat.com</a>>>> wrote:<br>
> > >>><br>
> > >>> Hello,<br>
> > >>><br>
> > >>> It has been recently requested that the API provides event<br>
> types:<br>
> > >>><br>
> > >>> [RFE] Expose event types to API<br>
> > >>> <a href="https://bugzilla.redhat.com/1453170" rel="noreferrer" target="_blank">https://bugzilla.redhat.com/<wbr>1453170</a><br>
> <<a href="https://bugzilla.redhat.com/1453170" rel="noreferrer" target="_blank">https://bugzilla.redhat.com/<wbr>1453170</a>><br>
> > <<a href="https://bugzilla.redhat.com/1453170" rel="noreferrer" target="_blank">https://bugzilla.redhat.com/<wbr>1453170</a><br>
> <<a href="https://bugzilla.redhat.com/1453170" rel="noreferrer" target="_blank">https://bugzilla.redhat.com/<wbr>1453170</a>>><br>
> > >>><br>
> > >>> Currently the API provides the event code and description, for<br>
> > example:<br>
> > >>><br>
> > >>> <event href="/ovirt-engine/api/<wbr>events/8021" id="8021"><br>
> > >>> <code>19</code><br>
> > >>> <description>Host myhost failed to recover.</description<br>
> > >>> ...<br>
> > >>> </event><br>
> > >>><br>
> > >>> There is no documentation of what is the meaning of codes,<br>
> > except the<br>
> > >>> source code of the engine itself. This forces some<br>
> applications<br>
> > to add<br>
> > >>> their own code to name mapping. For example, the 'ovirt' Ruby<br>
> > gem used<br>
> > >>> by older versions of ManageIQ to interact with oVirt contains<br>
> > the following:<br>
> > >>><br>
> > >>><br>
> ><br>
> <a href="https://github.com/ManageIQ/ovirt/blob/v0.17.0/lib/ovirt/event.rb#L25-L485" rel="noreferrer" target="_blank">https://github.com/ManageIQ/<wbr>ovirt/blob/v0.17.0/lib/ovirt/<wbr>event.rb#L25-L485</a><br>
> <<a href="https://github.com/ManageIQ/ovirt/blob/v0.17.0/lib/ovirt/event.rb#L25-L485" rel="noreferrer" target="_blank">https://github.com/ManageIQ/<wbr>ovirt/blob/v0.17.0/lib/ovirt/<wbr>event.rb#L25-L485</a>><br>
> ><br>
> <<a href="https://github.com/ManageIQ/ovirt/blob/v0.17.0/lib/ovirt/event.rb#L25-L485" rel="noreferrer" target="_blank">https://github.com/ManageIQ/<wbr>ovirt/blob/v0.17.0/lib/ovirt/<wbr>event.rb#L25-L485</a><br>
> <<a href="https://github.com/ManageIQ/ovirt/blob/v0.17.0/lib/ovirt/event.rb#L25-L485" rel="noreferrer" target="_blank">https://github.com/ManageIQ/<wbr>ovirt/blob/v0.17.0/lib/ovirt/<wbr>event.rb#L25-L485</a>>><br>
> > >>><br>
> > >>> We could avoid this by adding to the API a new event<br>
> attribute that<br>
> > >>> indicates the type:<br>
> > >>><br>
> > >>> <event href="/ovirt-engine/api/<wbr>events/8021" id="8021"><br>
> > >>> <code>19</code><br>
> > >>> <type>host_recover_failure</<wbr>type><br>
> > >>> <description>Host myhost failed to recover.</description><br>
> > >>> ...<br>
> > >>> </event><br>
> > >>><br>
> > >>> Ideally this should be defined as an enum, so that it will be<br>
> > >>> represented as an enum in the SDKs. Alternatively it could<br>
> just<br>
> > be an<br>
> > >>> string, and we could reuse the 'name' attribute:<br>
> > >>><br>
> > >>> <event href="/ovirt-engine/api/<wbr>events/8021" id="8021"><br>
> > >>> <code>19</code><br>
> > >>> <name>host_recover_failure</<wbr>name><br>
> > >>> <description>Host myhost failed to recover.</description><br>
> > >>> ...<br>
> > >>> </event><br></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail-h5">
> > >>><br>
> > >>> However, the key point to making this useful would be to keep<br>
> > the types<br>
> > >>> (or names) backwards compatible, so that users of the API can<br>
> > rely on<br>
> > >>> their values and meanings.<br>
> > >>><br>
> > >>> So this is my question to you: can we commit to keep the<br>
> names and<br>
> > >>> meanings of the backend event types backwards compatible?<br></div></div></blockquote><div><br><div>we can maintain the mapping layer between backend event types to restapi event types<br></div><div>on the restapi side. This is currently done on the ovirt-gem, and once ovirt-gem is depracated<br></div><div>we could rely the manageiq-providers-ovirt code on the api/sdk which will expose the event name.<br></div><div>Having a single api maintainer will ease the maintenance of it and will allow a better control of<br></div><div>changes to that part. <br></div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail-h5">
> > >><br>
> > >> Do we even have to make it bw compatible?<br>
> > >> I guess it depends on the actual usage of those names…<br>
> > >> The ovirt ruby gem itself doesn’t do much with it<br>
> > ><br>
> > > We need to make keep it backwards compatible or else tell<br>
> users "don't<br>
> > > rely on these values, as they may change without notice".<br>
> > ><br>
> > > The 'ovirt' gem doesn't do anything special, it just creates<br>
> its own<br>
> > > code to name mapping. But the users of the 'ovirt' gem (the<br>
> ManageIQ<br>
> > > oVirt provider) do rely on the name. For example:<br>
> > ><br>
> > ><br>
> > ><br>
> <a href="https://github.com/ManageIQ/manageiq-providers-ovirt/blob/master/app/models/manageiq/providers/redhat/infra_manager/event_parser.rb#L80-L92" rel="noreferrer" target="_blank">https://github.com/ManageIQ/<wbr>manageiq-providers-ovirt/blob/<wbr>master/app/models/manageiq/<wbr>providers/redhat/infra_<wbr>manager/event_parser.rb#L80-<wbr>L92</a><br>
> <<a href="https://github.com/ManageIQ/manageiq-providers-ovirt/blob/master/app/models/manageiq/providers/redhat/infra_manager/event_parser.rb#L80-L92" rel="noreferrer" target="_blank">https://github.com/ManageIQ/<wbr>manageiq-providers-ovirt/blob/<wbr>master/app/models/manageiq/<wbr>providers/redhat/infra_<wbr>manager/event_parser.rb#L80-<wbr>L92</a>><br>
> ><br>
> <<a href="https://github.com/ManageIQ/manageiq-providers-ovirt/blob/master/app/models/manageiq/providers/redhat/infra_manager/event_parser.rb#L80-L92" rel="noreferrer" target="_blank">https://github.com/ManageIQ/<wbr>manageiq-providers-ovirt/blob/<wbr>master/app/models/manageiq/<wbr>providers/redhat/infra_<wbr>manager/event_parser.rb#L80-<wbr>L92</a><br>
> <<a href="https://github.com/ManageIQ/manageiq-providers-ovirt/blob/master/app/models/manageiq/providers/redhat/infra_manager/event_parser.rb#L80-L92" rel="noreferrer" target="_blank">https://github.com/ManageIQ/<wbr>manageiq-providers-ovirt/blob/<wbr>master/app/models/manageiq/<wbr>providers/redhat/infra_<wbr>manager/event_parser.rb#L80-<wbr>L92</a>>><br>
> ><br>
> ><br>
> > hmmm, while we are on topic, this pretty much looks like that manageiq<br>
> > does not only rely on the code but also on the actual value of it<br>
> since<br>
> > it is parsing it:<br>
> ><br>
> > # sample message: "Interface nic1 (VirtIO) was added to VM v5. (User:<br>
> > admin@internal-authz)" message.split(/\s/)[7][0...-1]<br>
> ><br>
> > Is this something we commit to maintain? Or should we commit to<br>
> maintain it?<br>
> ><br>
><br>
> That is a good point, that isn't very future proof. We should also find<br>
> a way to make less fragile. Any suggestion?<br>
><br>
><br>
> The only doable thing which comes to my mind is something like this:<br>
> The msg is defined like this:<br>
> USER_ADD_VM_POOL_WITH_VMS_<wbr>FAILED=Failed to create VM Pool ${VmPoolName}<br>
> (User: ${UserName}).<br>
><br>
> e.g. the msg type and the variables. If we could expose in the api not<br>
> only the substituted msg but also the variable/value binding, we could<br>
> commit to keep the variable names backward compatible.<br>
><br>
> So, something like:<br>
><br>
> <event href="/ovirt-engine/api/<wbr>events/8021" id="8021"><br>
> <code>19</code><br>
> <type>USER_ADD_VM_POOL_WITH_<wbr>VMS_FAILED</type><br>
> <description>the substituted msg.</description><br>
> <parameters><br>
> <parameter><br>
> <key>VmPoolName</key><br>
> <value>The Pool Name<value><br>
> </parameter><br>
> ...<br>
> </parameters><br>
> </event><br>
><br>
> Not really rock solid since the variables would still be defined in the<br>
> AuditLogMessages.properties but still better and still easier to parse<br>
> on the client side.<br>
><br>
<br>
</div></div>That makes sense to me. I have opened the following bug to track that:<br>
<br>
[RFE] Add properties to events<br>
<a href="https://bugzilla.redhat.com/1456711" rel="noreferrer" target="_blank">https://bugzilla.redhat.com/<wbr>1456711</a><br>
<br></blockquote><div><br></div><div>I don't think we should expose the internal implementation of the audit-logging mechanism via the api.<br></div><div>This is unreliable and not the recommended way IMO to obtain data related to the event.<br><br></div><div>If the concern is for the entities names, we can extend the returned event element in the response<br>to include also the name:<br><br><div><div><div><event><br></div> <vm id="..." href="..."><br></div> <name>abc</name><br></div> </vm><br><br></div><div>This can be applied to all of the main entities without penalty in data retrieval from the database, since<br></div><div>the names of the entities should also be available on the audit-log entity.<br><br></div><div>However, for all of the other placesholders - this is a coupling of the restapi event structure to the<br>implementation of the auditlogging:<br></div><div>With the current code we don't store as part of the event data all of the raw material of the event, <br>specifically any variable being set via the AuditLogable.#customValues().<br></div><div>Therefore some of the data won't be available for the returned event.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Note that for the particular case of the VM name, which is what ManageIQ<br>
is trying to do in that code, the current best way is to use the <vm<br>
.../> link that is part of the event. I have opened the following<br>
ManageIQ issue to track it:<br>
<br>
Avoid parsing the descriptions of events<br>
<a href="https://github.com/ManageIQ/manageiq-providers-ovirt/issues/45" rel="noreferrer" target="_blank">https://github.com/ManageIQ/<wbr>manageiq-providers-ovirt/<wbr>issues/45</a><br>
<span class="gmail-"><br>
><br>
> ><br>
> > ><br>
> > > That means that if we ever change the meaning of a code the ManageIQ<br>
> > > provider, for example, will break.<br>
> ><br>
> > Right,then it indeed needs to stay stable.<br>
> > But how is maintaining the enum string different from the code? It is<br>
> > the same information, so if MIQ doesn't use the name directly then it<br>
> > doesn't really matter if it's a code or string.<br>
> > Perhaps deprecate the code and keep the name fixed?<br>
> ><br>
> > Thanks,<br>
> > michal<br>
> ><br>
> > ><br>
> > >>><br>
> > >>> Regards,<br>
> > >>> Juan Hernandez<br>
> > >>><br>
> > >>><br>
> > >>> ______________________________<wbr>_________________<br>
> > >>> Devel mailing list<br>
> > >>> <a href="mailto:Devel@ovirt.org">Devel@ovirt.org</a> <mailto:<a href="mailto:Devel@ovirt.org">Devel@ovirt.org</a>><br>
</span>> <mailto:<a href="mailto:Devel@ovirt.org">Devel@ovirt.org</a> <mailto:<a href="mailto:Devel@ovirt.org">Devel@ovirt.org</a>>><br>
<span class="gmail-">> > >>> <a href="http://lists.ovirt.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.ovirt.org/<wbr>mailman/listinfo/devel</a><br>
> <<a href="http://lists.ovirt.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.ovirt.org/<wbr>mailman/listinfo/devel</a>><br>
> > <<a href="http://lists.ovirt.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.ovirt.org/<wbr>mailman/listinfo/devel</a><br>
> <<a href="http://lists.ovirt.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.ovirt.org/<wbr>mailman/listinfo/devel</a>>><br>
> > ><br>
> > ______________________________<wbr>_________________<br>
> > Devel mailing list<br>
> > <a href="mailto:Devel@ovirt.org">Devel@ovirt.org</a> <mailto:<a href="mailto:Devel@ovirt.org">Devel@ovirt.org</a>><br>
</span>> <mailto:<a href="mailto:Devel@ovirt.org">Devel@ovirt.org</a> <mailto:<a href="mailto:Devel@ovirt.org">Devel@ovirt.org</a>>><br>
<span class="gmail-">> > <a href="http://lists.ovirt.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.ovirt.org/<wbr>mailman/listinfo/devel</a><br>
> <<a href="http://lists.ovirt.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.ovirt.org/<wbr>mailman/listinfo/devel</a>><br>
> > <<a href="http://lists.ovirt.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.ovirt.org/<wbr>mailman/listinfo/devel</a><br>
> <<a href="http://lists.ovirt.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.ovirt.org/<wbr>mailman/listinfo/devel</a>>><br>
> ><br>
> ><br>
><br>
><br>
<br>
______________________________<wbr>_________________<br>
Devel mailing list<br>
<a href="mailto:Devel@ovirt.org">Devel@ovirt.org</a><br>
</span><a href="http://lists.ovirt.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.ovirt.org/<wbr>mailman/listinfo/devel</a></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature"><div dir="ltr"><div>Regards,<br></div>Moti<br></div></div>
</div></div></div></div></div></div>