| Message ID | 20210909080547.1331581-3-punitagrawal@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | rteval: Miscellaneous fixes | expand |
On Thu, 9 Sep 2021, Punit Agrawal wrote: > From: Punit Agrawal <punit1.agrawal@toshiba.co.jp> > > rteval mistakenly detects that it is running inside a container even > though it is running directly on the host. On further investigation > this was found to be due to change in behaviour around byte strings > and strings when going from python2 to python3. > > In python3 byte strings are not equivalent to strings, i.e., b'' == '' > is False. The string comparison functions in services.py are still > relying on the old behaviour in python2 where they were equivalent. > > Update the byte string processing by converting them to string. > > Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp> > --- > rteval/sysinfo/services.py | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/rteval/sysinfo/services.py b/rteval/sysinfo/services.py > index 06ff5ae9cd0c..94857aea6be4 100644 > --- a/rteval/sysinfo/services.py > +++ b/rteval/sysinfo/services.py > @@ -83,8 +83,8 @@ class SystemServices: > self.__log(Log.DEBUG, "cmd: %s" % cmd) > c = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) > for p in c.stdout: > - # p are lines like "servicename.service status" > - v = p.strip().split() > + # p are lines like b'servicename.service status' > + v = p.decode().strip().split() > ret_services[v[0].split('.')[0]] = v[1] > return ret_services > > @@ -92,7 +92,7 @@ class SystemServices: > def services_get(self): > cmd = [getcmdpath('ps'), '-ocomm=', '1'] > c = subprocess.Popen(cmd, stdout=subprocess.PIPE) > - self.__init = c.stdout.read().strip() > + self.__init = c.stdout.read().decode().strip() > if self.__init == 'systemd': > self.__log(Log.DEBUG, "Using systemd to get services status") > return self.__get_services_systemd() > -- > 2.32.0 > > Thanks, this looks good, I'm wondering however if the same result could be achieved by appending text=True to the subprocess command in each of those methods? Would you like to test that and send me a new patch? Thanks John
John Kacur <jkacur@redhat.com> writes: > On Thu, 9 Sep 2021, Punit Agrawal wrote: > >> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp> >> >> rteval mistakenly detects that it is running inside a container even >> though it is running directly on the host. On further investigation >> this was found to be due to change in behaviour around byte strings >> and strings when going from python2 to python3. >> >> In python3 byte strings are not equivalent to strings, i.e., b'' == '' >> is False. The string comparison functions in services.py are still >> relying on the old behaviour in python2 where they were equivalent. >> >> Update the byte string processing by converting them to string. >> >> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp> >> --- >> rteval/sysinfo/services.py | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/rteval/sysinfo/services.py b/rteval/sysinfo/services.py >> index 06ff5ae9cd0c..94857aea6be4 100644 >> --- a/rteval/sysinfo/services.py >> +++ b/rteval/sysinfo/services.py >> @@ -83,8 +83,8 @@ class SystemServices: >> self.__log(Log.DEBUG, "cmd: %s" % cmd) >> c = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) >> for p in c.stdout: >> - # p are lines like "servicename.service status" >> - v = p.strip().split() >> + # p are lines like b'servicename.service status' >> + v = p.decode().strip().split() >> ret_services[v[0].split('.')[0]] = v[1] >> return ret_services >> >> @@ -92,7 +92,7 @@ class SystemServices: >> def services_get(self): >> cmd = [getcmdpath('ps'), '-ocomm=', '1'] >> c = subprocess.Popen(cmd, stdout=subprocess.PIPE) >> - self.__init = c.stdout.read().strip() >> + self.__init = c.stdout.read().decode().strip() >> if self.__init == 'systemd': >> self.__log(Log.DEBUG, "Using systemd to get services status") >> return self.__get_services_systemd() >> -- >> 2.32.0 >> >> > > Thanks, this looks good, I'm wondering however if the same result could be > achieved by appending text=True to the subprocess command in each of those > methods? Would you like to test that and send me a new patch? Thanks for the suggestion - I missed the "text=True" in the Popen() when going through the documentation. I will send a new version converting all Popen() sites in the file. [...]
On Mon, 13 Sep 2021, Punit Agrawal wrote: > From: Punit Agrawal <punit1.agrawal@toshiba.co.jp> > > rteval mistakenly detects that it is running inside a container even > though it is running directly on the host. On further investigation > this was found to be due to change in behaviour around byte strings > and strings when going from python2 to python3. > > In python3 process output (stdout, stderr) for processes executed via > Popen() is retuned as byte strings and byte strings are not equivalent > to strings, i.e., b'' == '' is False. > > Update all call sites for Popen() to include "text=True" > argument - this ensures that the process output is returned as a > string. > > Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp> > --- > v1 -> v2: > * Update Popen() calls to return strings for process output > --- > rteval/sysinfo/services.py | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/rteval/sysinfo/services.py b/rteval/sysinfo/services.py > index 06ff5ae9cd0c..283109b9b234 100644 > --- a/rteval/sysinfo/services.py > +++ b/rteval/sysinfo/services.py > @@ -62,11 +62,11 @@ class SystemServices: > if not [1 for p in reject if fnmatch.fnmatch(servicename, p)] \ > and os.access(service, os.X_OK): > cmd = '%s -qs "\(^\|\W\)status)" %s' % (getcmdpath('grep'), service) > - c = subprocess.Popen(cmd, shell=True) > + c = subprocess.Popen(cmd, shell=True, text=True) > c.wait() > if c.returncode == 0: > cmd = ['env', '-i', 'LANG="%s"' % os.environ['LANG'], 'PATH="%s"' % os.environ['PATH'], 'TERM="%s"' % os.environ['TERM'], service, 'status'] > - c = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) > + c = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) > c.wait() > if c.returncode == 0 and (c.stdout.read() or c.stderr.read()): > ret_services[servicename] = 'running' > @@ -81,9 +81,9 @@ class SystemServices: > ret_services = {} > cmd = '%s list-unit-files -t service --no-legend' % getcmdpath('systemctl') > self.__log(Log.DEBUG, "cmd: %s" % cmd) > - c = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) > + c = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) > for p in c.stdout: > - # p are lines like "servicename.service status" > + # p are lines like b'servicename.service status' > v = p.strip().split() > ret_services[v[0].split('.')[0]] = v[1] > return ret_services > @@ -91,7 +91,7 @@ class SystemServices: > > def services_get(self): > cmd = [getcmdpath('ps'), '-ocomm=', '1'] > - c = subprocess.Popen(cmd, stdout=subprocess.PIPE) > + c = subprocess.Popen(cmd, stdout=subprocess.PIPE, text=True) > self.__init = c.stdout.read().strip() > if self.__init == 'systemd': > self.__log(Log.DEBUG, "Using systemd to get services status") > -- > 2.32.0 > > Signed-off-by: John Kacur <jkacur@redhat.com>
diff --git a/rteval/sysinfo/services.py b/rteval/sysinfo/services.py index 06ff5ae9cd0c..94857aea6be4 100644 --- a/rteval/sysinfo/services.py +++ b/rteval/sysinfo/services.py @@ -83,8 +83,8 @@ class SystemServices: self.__log(Log.DEBUG, "cmd: %s" % cmd) c = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) for p in c.stdout: - # p are lines like "servicename.service status" - v = p.strip().split() + # p are lines like b'servicename.service status' + v = p.decode().strip().split() ret_services[v[0].split('.')[0]] = v[1] return ret_services @@ -92,7 +92,7 @@ class SystemServices: def services_get(self): cmd = [getcmdpath('ps'), '-ocomm=', '1'] c = subprocess.Popen(cmd, stdout=subprocess.PIPE) - self.__init = c.stdout.read().strip() + self.__init = c.stdout.read().decode().strip() if self.__init == 'systemd': self.__log(Log.DEBUG, "Using systemd to get services status") return self.__get_services_systemd()