Re: [libvirt] [PATCH python 2/2] event-test: Add storage pool lifecycle event tests

On 06/14/2016 03:45 AM, Martin Kletzander wrote: > On Mon, Jun 13, 2016 at 12:51:19PM -0400, Cole Robinson wrote: >> On 06/11/2016 01:43 PM, Jovanka Gulicoska wrote: >>> --- >>> examples/event-test.py | 18 ++++++++++++++++++ >>> 1 file changed, 18 insertions(+) >>> >>> diff --git a/examples/event-test.py b/examples/event-test.py >>> index f96c917..241369b 100755 >>> --- a/examples/event-test.py >>> +++ b/examples/event-test.py >>> @@ -565,6 +565,23 @@ def myNetworkEventLifecycleCallback(conn, net, event, >>> detail, opaque): >>> >>> netDetailToString(event, detail))) >>> >>> ########################################################################## >>> +# Storage pool events >>> +########################################################################## >>> +def storageEventToString(event): >>> + storageEventStrings = ( "Defined", >>> + "Undefined", >>> + "Started", >>> + "Stopped", >>> + "Refreshed", >>> + ) >>> + return storageEventStrings[event] >>> + >> >> This pattern is problematic, since if libvirt grows a new event ID this code >> throws an exception until we extend storageEventStrings. That said, this >> pattern is used all over event-test.py so it deserves a larger cleanup IMO, >> there's even a bug for an issue like this >> >> https://bugzilla.redhat.com/show_bug.cgi?id=1274145 >> > > Well, it's an example, so I think it's fine. It's under examples/ but it serves a more interesting purpose of being the simplest way to validate that new python event APIs are working correctly. So removing a pattern that makes it less noisy and crashy is a worthy goal IMO

Thanks, Cole