On Fri, Oct 30, 2020 at 01:25:47PM +0100, Jan Tluka wrote:
Fri, Oct 30, 2020 at 12:59:06PM CET, olichtne(a)redhat.com wrote:
>On Fri, Oct 30, 2020 at 09:30:45AM +0100, Jan Tluka wrote:
>> The method will be used by mixin classes derived from both
>> BasePerfTestTweakMixin and BasePerfTestIterationTweakMixin.
>>
>> Signed-off-by: Jan Tluka <jtluka(a)redhat.com>
>> ---
>> .../Perf/PerfTestMixins/BasePerfTestTweakMixin.py | 6 ------
>> .../ENRT/PerfTestMixins/SctpFirewallPerfTestMixin.py | 5 +++--
>> lnst/Recipes/ENRT/PerfTestMixins/Utils.py | 5 +++++
>> 3 files changed, 8 insertions(+), 8 deletions(-)
>> create mode 100644 lnst/Recipes/ENRT/PerfTestMixins/Utils.py
>>
>> diff --git a/lnst/RecipeCommon/Perf/PerfTestMixins/BasePerfTestTweakMixin.py
b/lnst/RecipeCommon/Perf/PerfTestMixins/BasePerfTestTweakMixin.py
>> index 5e008a68..380fc72e 100644
>> --- a/lnst/RecipeCommon/Perf/PerfTestMixins/BasePerfTestTweakMixin.py
>> +++ b/lnst/RecipeCommon/Perf/PerfTestMixins/BasePerfTestTweakMixin.py
>> @@ -1,5 +1,3 @@
>> -from lnst.RecipeCommon.Perf.Measurements.BaseFlowMeasurement import
BaseFlowMeasurement
>> -
>> class BasePerfTestTweakMixin(object):
>> """
>> This is a base class that defines common API for specific *perf test*
>> @@ -15,7 +13,3 @@ class BasePerfTestTweakMixin(object):
>> def remove_perf_test_tweak(self, perf_config):
>> # TODO: check if anything left in the
perf_config.perf_test_tweak_config
>> pass
>> -
>> - def _get_flow_measurement_from_config(self, perf_config):
>> - flow_measurements = [ m for m in perf_config.measurements if
isinstance(m, BaseFlowMeasurement) ]
>> - return flow_measurements[0]
>> diff --git a/lnst/Recipes/ENRT/PerfTestMixins/SctpFirewallPerfTestMixin.py
b/lnst/Recipes/ENRT/PerfTestMixins/SctpFirewallPerfTestMixin.py
>> index 841df3f7..42cbf492 100644
>> --- a/lnst/Recipes/ENRT/PerfTestMixins/SctpFirewallPerfTestMixin.py
>> +++ b/lnst/Recipes/ENRT/PerfTestMixins/SctpFirewallPerfTestMixin.py
>> @@ -1,11 +1,12 @@
>> from lnst.Controller.RecipeResults import ResultLevel
>> from lnst.RecipeCommon.Perf.PerfTestMixins import BasePerfTestTweakMixin
>> +from lnst.Recipes.ENRT.PerfTestMixins.Utils import
get_flow_measurement_from_config
>>
>> class SctpFirewallPerfTestMixin(BasePerfTestTweakMixin):
>> def apply_perf_test_tweak(self, perf_config):
>> super().apply_perf_test_tweak(perf_config)
>>
>> - flow_measurement = self._get_flow_measurement_from_config(perf_config)
>> + flow_measurement = get_flow_measurement_from_config(perf_config)
>> flow = flow_measurement.conf[0]
>> if flow.type == "sctp_stream":
>> for nic in [flow.generator_nic, flow.receiver_nic]:
>> @@ -18,7 +19,7 @@ class SctpFirewallPerfTestMixin(BasePerfTestTweakMixin):
>> tweak_config["iptables_sctp"] = True
>>
>> def remove_perf_test_tweak(self, perf_config):
>> - flow_measurement = self._get_flow_measurement_from_config(perf_config)
>> + flow_measurement = get_flow_measurement_from_config(perf_config)
>> flow = flow_measurement.conf[0]
>> if flow.type == "sctp_stream":
>> for nic in [flow.generator_nic, flow.receiver_nic]:
>> diff --git a/lnst/Recipes/ENRT/PerfTestMixins/Utils.py
b/lnst/Recipes/ENRT/PerfTestMixins/Utils.py
>> new file mode 100644
>> index 00000000..9f6f167e
>> --- /dev/null
>> +++ b/lnst/Recipes/ENRT/PerfTestMixins/Utils.py
>> @@ -0,0 +1,5 @@
>> +from lnst.RecipeCommon.Perf.Measurements.BaseFlowMeasurement import
BaseFlowMeasurement
>> +
>> +def get_flow_measurement_from_config(perf_config):
>> + flow_measurements = [ m for m in perf_config.measurements if isinstance(m,
BaseFlowMeasurement) ]
>> + return flow_measurements[0]
>
>I'm a bit worried about extending the use of the function and also
>burying it "deeper" into a "utils" module while the function
returns a
>specific [0] value.
>
>I know that currently we only have a single FlowMeasurement in the list,
>but if we ever add more than one, this could become quite a complicated
>thing to find/debug.
>
>Would it make more sense to have this as a "get_...measurementS..."
>method, return all of the flow measurements and then, for now, pick the
>[0] directly the the place where it's being used?
>
Makes sense. So, when you say to have this as a method, you mean a class
method, correct? I will need to create a base class for PerfTest and
PerfTestIteration classes that would share this method (which is ok).
Sorry, got my method vs function words confused, I'm ok with keeping
this as a function in the Utils module. No need for a new base class, I
don't think it's a good idea to merge distinct mixin classes with a
common base class...
-Ondrej
>Optionally maybe renaming the method to get_first_flow_measurement...
>could also work and make it more obvious what's happening.
>
>-Ondrej
>
>> --
>> 2.21.3
>> _______________________________________________
>> LNST-developers mailing list -- lnst-developers(a)lists.fedorahosted.org
>> To unsubscribe send an email to lnst-developers-leave(a)lists.fedorahosted.org
>> Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>> List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
>> List Archives:
https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedora...