Mon, Jul 22, 2019 at 04:45:12PM CEST, csfakian(a)redhat.com wrote:
Hi,
----- Original Message -----
> From: "Ondrej Lichtner" <olichtne(a)redhat.com>
> To: csfakian(a)redhat.com
> Cc: lnst-developers(a)lists.fedorahosted.org
> Sent: Monday, July 22, 2019 3:48:05 PM
> Subject: Re: [PATCH-next 7/8] lnst.Devices.OvsBridgeDevice: add config getters
>
> On Fri, Jul 19, 2019 at 01:36:50PM +0200, csfakian(a)redhat.com wrote:
> > From: Christos Sfakianakis <csfakian(a)redhat.com>
> > + @property
> > + def flows(self):
> > + flows = []
> > + ignore_exprs = [r"cookie", r"duration",
r"n_packets",
> > + r"n_bytes", r"idle_age"]
> > + out = exec_cmd("ovs-ofctl dump-flows %s" % self.name,
> > + log_outputs=False)[0]
> > +
> > + for line in out.split('\n'):
> > + if line:
> > + flows.append(line.split(', '))
> > +
> > + for flow in flows[1:]:
> > + for entry in list(flow):
> > + for expr in ignore_exprs:
> > + if re.search(expr, entry):
> > + del flow[flow.index(entry)]
> > + break
> > +
> > + return pprint.pformat(flows[1:])
>
> The previous properties return a data structure (a dict), but this one
> returns a formatted string, is that intentional? If so would it make
> more sense to name the method with something more descriptive e.g.
> flows_desc or flows_str. Or if not then returning a data structure would
> be better?
>
This was intentional, I tried to filter out non-relevant info from the output,
while still storing it line by line. I will name the method differently.
When you say non-relevant I assume the code would work even without
filtering. Is this really necessary or just some sort of optimization?
And I wonder if we find out that we need some of the 'non-relevant' data
some time in the future.
If the code works without filtering I think it should be removed.
-Jan