This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/805/ |
On September 2nd, 2013, 11:58 a.m. UTC, Radek Novacek wrote:
src/service-dbus/test/testService.py (Diff revision 1) 100 sysd_output = subprocess.check_output("systemctl list-unit-files -t service --no-pager --no-legend | grep -v \@\.service", shell=True)101 sysd_services_tmp = filter (None, map (lambda x: x.split(), sysd_output.split("\n")))102 sysd_services = dict((s[0], False) for (s) in sysd_services_tmp)This is way too complicated only to get list of services (and also hard to read). I would go with simple loop like this: sysd_services = [] for line in subprocess.check_output(["/usr/bin/systemctl", "list-unit-files", "-t", "service", "--no-pager", "--no-legend"]).split("\n"): service_name = line.split(" ", 1)[0] if service_name and "@.service" not in service_name: sysd_services.append(service_name)
It's not that hard to read when you know basics of functional programming. Maybe grep in subprocess.check_output can be replaced by better solution. I don't like something like: for line in some_very_long_function_name_with(lot, of, parameters, howg):
On September 2nd, 2013, 11:58 a.m. UTC, Radek Novacek wrote:
src/service-dbus/test/testService.py (Diff revision 1) 114 sysd_state = True if (ret == 0) else Falsesysd_state = ret == 0On September 3rd, 2013, 7:08 a.m. UTC, Robin Hack wrote:
It's more harder to read without parentheses.On September 3rd, 2013, 7:14 a.m. UTC, Radek Novacek wrote:
I'm ok with parentheses but not with the ternary operator.
Fixed with: sysd_state = (ret == 0)
- Robin
On September 2nd, 2013, 11:16 a.m. UTC, Robin Hack wrote:
Review request for OpenLMI Developers.
By Robin Hack.
Updated Sept. 2, 2013, 11:16 a.m.
Repository:
openlmi-providers
Description
Diffs
|