Its not just that it avoids dumping the samples, it prevents creating the sample file in the first place. 

I used the null context here mostly because it just makes the code flow simpler, due to the fact you can't really wrap a "with" in a conditional. I challenge you to rewrite that code block another way, and have it come out nicely. 

I considered a couple alternatives, like using this contextlib2 (https://github.com/jazzband/contextlib2). But since the code was so simple it was easier to just copy it instead of adding a new dependency. 

On Tue, Apr 6, 2021 at 3:41 AM Jan Tluka <jtluka@redhat.com> wrote:
Thu, Apr 01, 2021 at 09:13:37PM CEST, pgagne@redhat.com wrote:
>From: Perry Gagne <pgagne@redhat.com>
>
>Neper tcp_crr removed sample stats from the server due to memory issues.
>
>We decided to not look at the server side stats for now, and just record it
>as a PerfInterval of 0.
>
>Signed-off-by: Perry Gagne <pgagne@redhat.com>
>---
> .../Perf/Measurements/NeperFlowMeasurement.py |  8 +++++
> lnst/Tests/Neper.py                           | 29 ++++++++++++++-----
> 2 files changed, 29 insertions(+), 8 deletions(-)
>
>diff --git a/lnst/RecipeCommon/Perf/Measurements/NeperFlowMeasurement.py b/lnst/RecipeCommon/Perf/Measurements/NeperFlowMeasurement.py
>index 1f667fb5..f271e1b3 100644
>--- a/lnst/RecipeCommon/Perf/Measurements/NeperFlowMeasurement.py
>+++ b/lnst/RecipeCommon/Perf/Measurements/NeperFlowMeasurement.py
>@@ -171,6 +171,14 @@ class NeperFlowMeasurement(BaseFlowMeasurement):
>         if not job.passed:
>             results.append(PerfInterval(0, 0, "transactions", time.time()))
>             cpu_results.append(PerfInterval(0, 0, "cpu_percent", time.time()))
>+        elif job.what.is_crr_server():
>+            # Neper doesn't support server stats for tcp_crr due to memory issues.
>+            # Use perf_interval of 0.
>+            # For duration neper doesn't have time_start/time_end for tcp_crr
>+            # So just use the value of test length
>+            d = float(job.what.params.test_length)
>+            results.append(PerfInterval(0, d, "transactions", time.time()))
>+            cpu_results.append(PerfInterval(0, d, "cpu_percent", time.time()))
>         else:
>             job_start = job.result['start_time']
>             samples = job.result['samples']
>diff --git a/lnst/Tests/Neper.py b/lnst/Tests/Neper.py
>index 3c2b6445..1cdadf5d 100644
>--- a/lnst/Tests/Neper.py
>+++ b/lnst/Tests/Neper.py
>@@ -1,3 +1,4 @@
>+import contextlib
> import csv
> import logging
> import os
>@@ -6,9 +7,10 @@ import re
> import subprocess
> import time
> import tempfile
>-from typing import List, Dict
>+from typing import List, Dict, TextIO, Union
>
> from lnst.Common.Parameters import HostnameOrIpParam, StrParam, IntParam, IpParam, ChoiceParam
>+from lnst.Common.Utils import nullcontext
> from lnst.Tests.BaseTestModule import BaseTestModule, TestModuleError
>
> NEPER_OUT_RE = re.compile(r"^(?P<key>.*)=(?P<value>.*)$", flags=re.M)
>@@ -43,6 +45,9 @@ class NeperBase(BaseTestModule):
>                 data[k] = v
>         return data
>
>+    def is_crr_server(self):
>+        return self._role == "server" and self.params.workload == "tcp_crr"
>+
>     def run(self):
>         self._res_data = {}
>         if not NEPER_PATH.joinpath(self.params.workload).exists():
>@@ -51,10 +56,15 @@ class NeperBase(BaseTestModule):
>             logging.error(self._res_data['msg'])
>             return False
>
>-        with tempfile.NamedTemporaryFile('r', prefix='neper-samples-',
>-                                         suffix='.csv', newline='') as sf:
>+        if self.is_crr_server():
>+            cm = nullcontext()
>+        else:
>+            cm = tempfile.NamedTemporaryFile('r', prefix='neper-samples-',
>+                                         suffix='.csv', newline='')
>+
>+        with cm as sf:
>
>-            cmd = self._compose_cmd(sf.name)
>+            cmd = self._compose_cmd(sf)
>             logging.debug(f"compiled command: {cmd}")
>             logging.debug(f"running as {self._role}")
>
>@@ -78,13 +88,16 @@ class NeperBase(BaseTestModule):
>                 logging.error(self._res_data["msg"])
>                 return False
>
>-            self._res_data["samples"] = [r for r in csv.DictReader(sf)]
>+            if not self.is_crr_server():
>+                self._res_data["samples"] = [r for r in csv.DictReader(sf)]

Please correct me if I'm wrong, but if I understand this correctly, the
only reason to use nullcontext manager is to avoid dumping the samples?
Is there any other reason why to use it?

Eventually when upstream neper issue is fixed we would use the file so
maybe this is unnecessary.

I'm quite unsure about copying code for feature that is not present in
python3.6 just because of such change.

But I may be missing something.

-Jan

>
>         return True
>
>-    def _compose_cmd(self, samples_path:str) -> str:
>-        cmd = [f"./{self.params.workload}",
>-               f"--all-samples={samples_path}"]
>+    def _compose_cmd(self, sample_file: Union[TextIO, None]) -> str:
>+        cmd = [f"./{self.params.workload}"]
>+
>+        if not self.is_crr_server():
>+            cmd.append(f"--all-samples={sample_file.name}")
>
>         if self._role == "client":
>             cmd.append("-c")
>--
>2.30.2
>_______________________________________________
>LNST-developers mailing list -- lnst-developers@lists.fedorahosted.org
>To unsubscribe send an email to lnst-developers-leave@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.fedorahosted.org
>Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure