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)

On September 3rd, 2013, 7:20 a.m. UTC, Robin Hack wrote:

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 3rd, 2013, 7:35 a.m. UTC, Radek Novacek wrote:

Then store the result of some_very_long_function_name_with function to temporary variable.

On September 3rd, 2013, 8:34 a.m. UTC, Robin Hack wrote:

What about:
sysd_output = subprocess.check_output("systemctl list-unit-files -t service --no-pager --no-legend".split())
sysd_services_tmp = filter (lambda p: p and "@.service" not in p[0], map (lambda x: x.split(), sysd_output.split("\n")))

It's simpler. I removed the ugly grep and pipe.

On September 3rd, 2013, 8:48 a.m. UTC, Radek Novacek wrote:

A little bit better :) Add comment what it does and it's good to go.

On September 3rd, 2013, 8:50 a.m. UTC, Jan Synacek wrote:

Please, don't do this. This is impossible to read without staring at it for a while.
Final version:

        #  Parse list of services. Filter blank lines and services with @.service string in name.
        sysd_services = []
        for line in sysd_output.split("\n"):
            service = line.split()
            if service and "@.service" not in service[0]:
                sysd_services.append(service)

- 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

Service tests: Upstream test for service profider from QA.

Diffs

  • src/service-dbus/test/failing.service (PRE-CREATION)
  • src/service-dbus/test/testService.py (PRE-CREATION)

View Diff