Re: [RFC PATCH 3/8] create lnst.Recipes.ENRT.MeasurementGenerators

[Jan Tluka]

Mon, Jan 04, 2021 at 10:52:34AM CET, olichtne(a)redhat.com wrote:
> On Mon, Dec 21, 2020 at 01:40:56PM +0100, Jan Tluka wrote:
> > Fri, Dec 18, 2020 at 02:21:23PM CET, olichtne(a)redhat.com wrote:
> > >From: Ondrej Lichtner <olichtne(a)redhat.com&gt;
> > >
> > >Splitting off the perf measurement generation methods into a separate
> > >smaller package.
> > >
> > >This should make it easier to skip and define more custom behaviours for
> > >the various groups of recipes that we have. To provide more context -
> > >many of our recipes are baremetal and share the same characteristics
> > >wrt. flow generation, however for ovs+dpdk recipes or for virtual
> > >recipes, or for ipsec based recipes we need different groups of
> > >behaviours. These still share the same basic interface so some form of
> > >class inheritance is valid.
> > >
> > >These will replace the BaseEnrt generation mechanism.
> > >
> > >For now this commit adds 3 main generator types:
> > >* IperfMeasurementGenerator that generates iperf measurements using a
> > >  flow generation algorithm so that multiple combinations of
> > >  ip_protocol*transport_protocol flows are created for each provided
> > >  flow endpoint pair
> > >* FlowEndpointsStatCPUMeasurementGenerator that generates a cpu
> > >  measurement based on the endpoints of any flow measurements generated
> > >  before this
> > >* HypervisorsStatCPUMeasurementGenerator that generates a cpu
> > >  measurement based on a "hypervisors" property that should return the
> > >  list of hypervisor hosts in the recipe, this is intended to be used
> > >  with recipes that deal with virtual machines
> > >
> > >Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com&gt;
> > >---
> > > .../BaseMeasurementGenerator.py               |   3 +
> > > ...lowEndpointsStatCPUMeasurementGenerator.py |  26 ++++
> > > .../HypervisorsStatCPUMeasurementGenerator.py |  19 +++
> > > .../IperfMeasurementGenerator.py              | 146 ++++++++++++++++++
> > > .../ENRT/MeasurementGenerators/__init__.py    |   0
> > > 5 files changed, 194 insertions(+)
> > > create mode 100644
> lnst/Recipes/ENRT/MeasurementGenerators/BaseMeasurementGenerator.py
> > > create mode 100644
> lnst/Recipes/ENRT/MeasurementGenerators/FlowEndpointsStatCPUMeasurementGenerator.py
> > > create mode 100644
> lnst/Recipes/ENRT/MeasurementGenerators/HypervisorsStatCPUMeasurementGenerator.py
> > > create mode 100644
> lnst/Recipes/ENRT/MeasurementGenerators/IperfMeasurementGenerator.py
> > > create mode 100644 lnst/Recipes/ENRT/MeasurementGenerators/__init__.py
> > >
> > >diff --git a/lnst/Recipes/ENRT/MeasurementGenerators/BaseMeasurementGenerator.py
> b/lnst/Recipes/ENRT/MeasurementGenerators/BaseMeasurementGenerator.py
> > >new file mode 100644
> > >index 0000000..c13bfb7
> > >--- /dev/null
> > >+++ b/lnst/Recipes/ENRT/MeasurementGenerators/BaseMeasurementGenerator.py
> > >@@ -0,0 +1,3 @@
> > >+class BaseMeasurementGenerator(object):
> > >+    def generate_perf_measurements_combinations(self, config):
> > >+        return []
> > 
> > I'd simply name this generate_measurements_combinations (remove the
> > "perf" from the name). Without removing it it makes me think this is
> > about the perf_tool measurements but when I look at the changes it's
> > shared by both the cpu and perf tool measurement classes.
>
> I chose the name "generate_perf_measurements_combinations" because:
>
> * the BaseMeasurement class is in the lnst.RecipeCommon.Perf.Measurements package
> * the enrt recipes use "net_perf_tool" and "cpu_perf_tool"
> parameters,
>   not just "perf_tool" parameter
>
> But I get what you mean, I'll try and see how it looks or if it can be
> improved in a different way.
>
> -Ondrej
>  
With your explanation that makes sense. I think you can keep the name as
it is.

J.