Re: [ovirt-devel] Making event types backwards compatible?
On 29 May 2017, at 11:44, Juan Hernández <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> wrote:
Hello,
It has been recently requested that the API provides event types:
[RFE] Expose event types to API 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
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/...
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 http://lists.ovirt.org/mailman/listinfo/devel
On Tue, May 30, 2017 at 7:20 AM, Michal Skrivanek <mskrivan@redhat.com> wrote:
On 29 May 2017, at 11:44, Juan Hernández <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> wrote:
Hello,
It has been recently requested that the API provides event types:
[RFE] Expose event types to API 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
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
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 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 http://lists.ovirt.org/mailman/listinfo/devel
_______________________________________________ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel
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>> wrote:
> On 29 May 2017, at 11:44, Juan Hernández <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>> 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/... <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?
> > 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> >>> http://lists.ovirt.org/mailman/listinfo/devel <http://lists.ovirt.org/mailman/listinfo/devel> > _______________________________________________ Devel mailing list Devel@ovirt.org <mailto:Devel@ovirt.org> http://lists.ovirt.org/mailman/listinfo/devel <http://lists.ovirt.org/mailman/listinfo/devel>
On Tue, May 30, 2017 at 9:16 AM, Juan Hernández <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>> wrote:
> On 29 May 2017, at 11:44, Juan Hernández <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>> 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
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
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 >>> Devel@ovirt.org <mailto:Devel@ovirt.org> >>> http://lists.ovirt.org/mailman/listinfo/devel <http://lists.ovirt.org/mailman/listinfo/devel> > _______________________________________________ Devel mailing list Devel@ovirt.org <mailto:Devel@ovirt.org> http://lists.ovirt.org/mailman/listinfo/devel <http://lists.ovirt.org/mailman/listinfo/devel>
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/... <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/... <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
> > > > > 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>> > >
On Tue, May 30, 2017 at 11:28 AM, 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
event.rb#L25-L485>
> <https://github.com/ManageIQ/ovirt/blob/v0.17.0/lib/ovirt/
event.rb#L25-L485
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?
we can maintain the mapping layer between backend event types to restapi event types on the restapi side. This is currently done on the ovirt-gem, and once ovirt-gem is depracated we could rely the manageiq-providers-ovirt code on the api/sdk which will expose the event name. Having a single api maintainer will ease the maintenance of it and will allow a better control of changes to that part.
> >> > >> 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
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
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
I don't think we should expose the internal implementation of the audit-logging mechanism via the api. This is unreliable and not the recommended way IMO to obtain data related to the event. If the concern is for the entities names, we can extend the returned event element in the response to include also the name: <event> <vm id="..." href="..."> <name>abc</name> </vm> This can be applied to all of the main entities without penalty in data retrieval from the database, since the names of the entities should also be available on the audit-log entity. However, for all of the other placesholders - this is a coupling of the restapi event structure to the implementation of the auditlogging: With the current code we don't store as part of the event data all of the raw material of the event, specifically any variable being set via the AuditLogable.#customValues(). Therefore some of the data won't be available for the returned event.
Note that for the particular case of the VM name, which is what ManageIQ is trying to do in that code, 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
> > > > > 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
-- Regards, Moti
On 05/30/2017 07:20 AM, Michal Skrivanek wrote:
On 29 May 2017, at 11:44, Juan Hernández <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> wrote:
Hello,
It has been recently requested that the API provides event types:
[RFE] Expose event types to API 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
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/...
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.
It is true that we could commit to keep the meanings of numeric codes backwards compatible, and document them somehow. If we commit to do that then I'd say the issue is mostly solved. However, if we use an enum we have the additional advantage it is more readable (in the SDKs in particular) and that the Java compiler would help us to detect potential backwards compatibility breaking changes. In addition changes to the enum would need to be done and reviewed in the specification of the API, which would give us (us == the API maintainers) the opportunity to review/discuss/document/decide about them explicitly.
Perhaps deprecate the code and keep the name fixed?
Yes, that is the idea, if we commit to keep them stable, see the bug.
participants (4)
-
Juan Hernández -
Michal Skrivanek -
Moti Asayag -
Tomas Jelinek