[ovs-dev] [python 2/7] python: Create new vlog module.

Ethan Jackson ethan at nicira.com
Tue Sep 27 12:06:15 PDT 2011


> Forbidding creating more Vlog modules after initializing Vlog seems
> like an odd limitation.

I basically do this so that I can have a handle on what configuration
of "any" module means.  This is much easier if you know what the set
of possible modules is because you can simply iterate over them.
Otherwise you'd have to maintain some sort of default settings
structure hand special case modifications to "any" to update this
default settings structure. Definitely possible, but seemed like an
inconvenience since I don't expect people to be creating vlog modules
dynamically at runtime.  Given that explanation, do you still think
it's worth implementing? If so, I'll change it.

> Why are a Vlog module's own settings stored in a dict in a class
> attribute?  Wouldn't an instance attribute be more natural?

This was a possibly incorrect design decision.  Basically, I want all
Vlog modules with the same module name to share the same settings.
The current design enforces this invariant by only allowing one entry
in the __mfl table per module. Alternatively, I would have to maintain
an __all_vlogs list which would contain a reference to each vlog
instance.  In this list there could be multiple vlog instances for a
given module which would need to be updated in set_level.  I'm
personally not sure which approach is cleaner, do you have a
preference?

> The C version of Vlog defaults to "info" for all facilities.

I misread the C code.  I'll update this.

> Any reason not to allow an optional backtrace for all log levels?
Not a particularly good one I suppose.  The backtrace only works when
in exception context which seems like an error kind of thing.  I
suppose I can allow it anywhere though.  I'll update the patch.

> The C version of vlog accepts only "ANY" (uppercase).  The Python
> version only accepts any case.  (It's probably best to just make the C
> version accept any case.)

I agree, I don't really want to fix the c code right now though.

> The Python code to set a level doesn't report syntax errors in its
> arguments but just silently fails.

I'll fix this up.

Ethan



More information about the dev mailing list