[sos-devel] [PATCH] ovirt-engine: new plugin for oVirt project
Sandro Bonazzola
sbonazzo at redhat.com
Thu Feb 6 13:58:03 UTC 2014
Il 04/02/2014 17:32, Sandro Bonazzola ha scritto:
> Il 04/02/2014 16:05, Bryn M. Reeves ha scritto:
>> On 02/04/2014 02:56 PM, Sandro Bonazzola wrote:
>>> + def setup(self):
>>> + if self.get_option('jbosstrace'):
>>> + proc = subprocess.Popen(
>>> + args=[
>>> + '/usr/bin/pgrep',
>>> + '-f',
>>> + 'jboss',
>>> + ],
>>> + stdout=subprocess.PIPE,
>>> + )
>>> + output, err = proc.communicate()
>>> + returncode = proc.returncode
>>
>> Any reason to open code this rather than use an existing interface e.g.
>> call_ext_prog()? This returns a tuple (status, output, runtime) which
>> should give you everything you need.
>>
>> We try to avoid plugins using things like Popen directly as it means we
>> have to make sure they all handle things like the environment and file
>> descriptor inheritance properly (see bz1051009 for e.g.).
>
> done
>
>>
>>> + jboss_pids = set()
>>> + if returncode == 0:
>>> + jboss_pids = set([int(x) for x in output.splitlines()])
>>> + proc = subprocess.Popen(
>>> + args=[
>>> + '/usr/bin/pgrep',
>>> + '-f',
>>> + 'ovirt-engine',
>>> + ],
>>> + stdout=subprocess.PIPE,
>>> + )
>>> + engine_output, err = proc.communicate()
>>
>> Ditto - no need for handrolled Popen here.
>>
>> Other than that the plugin looks pretty good - I can change these to use
>> call_ext_prog() if you like but it's probably easier for you to test the
>> changes directly than me.
>
> done this too, loosing stderr output in logs.
Any comment?
>
>
>>
>> Regards,
>> Bryn.
>>
>
>
--
Sandro Bonazzola
Better technology. Faster innovation. Powered by community collaboration.
See how it works at redhat.com
More information about the sos-devel
mailing list