ScottEdge
ScottEdge

Reputation: 87

Python and Pipe/Call

I have a python script that is causing some problems in the environment it's being run. I was told that it "does not release file pipes that it opens to read the data." I believe the problem is in the last line. I want to make sure I make the correct changes. This is running in Python 2.7 environment Thanks.

#!/usr/bin/env python

import subprocess
import urllib
from xml.dom import minidom

ZIPCODE = '06840'
TEMP_TYPE = 'f'
HVAC_ZONES = ['HVAC']
TSTAT = 5


WEATHER_URL = 'http://xml.weather.yahoo.com/forecastrss?p=' + ZIPCODE +'&u=' + TEMP_TYPE
WEATHER_NS = 'http://xml.weather.yahoo.com/ns/rss/1.0'

dom = minidom.parse(urllib.urlopen(WEATHER_URL))
ycondition = dom.getElementsByTagNameNS(WEATHER_NS, 'condition')[0]
CURRENT_OUTDOOR_TEMP = ycondition.getAttribute('temp')

for zone in HVAC_ZONES:
   i =0
   while i < TSTAT:
       i += 1
       subprocess.Popen(['/Users/RPM/Applications/RacePointMedia/sclibridge','writestate', zone + '.HVAC_controller.ThermostatCurrentRemoteTemperature'+ '_' + str(i),CURRENT_OUTDOOR_TEMP + TEMP_TYPE.upper()], stdin=subprocess.PIPE)

I know I need to add a close but wan't to make sure I don't have any other memory leaks. Would I need to add

subprocess.close

Would this be a better way to use subprocess.call? Do you need to release/close subprocess.call()?

#!/usr/bin/env python

import subprocess
import urllib
from xml.dom import minidom

ZIPCODE = '06457'
TEMP_TYPE = 'f'  # f - farhenheit c- celsius (case sensative)
HVAC_ZONES = ['HVAC', 'HVAC2']
TSTAT = 64


WEATHER_URL = 'http://xml.weather.yahoo.com/forecastrss?p=' + ZIPCODE +'&u=' + TEMP_TYPE
WEATHER_NS = 'http://xml.weather.yahoo.com/ns/rss/1.0'

dom = minidom.parse(urllib.urlopen(WEATHER_URL))
ycondition = dom.getElementsByTagNameNS(WEATHER_NS, 'condition')[0]
CURRENT_OUTDOOR_TEMP = ycondition.getAttribute('temp')
print(CURRENT_OUTDOOR_TEMP)

for zone in HVAC_ZONES:
   i =0
   while i < TSTAT:
       i += 1
       command = ['/Users/RPM/Applications/RacePointMedia/sclibridge','writestate', zone + '.HVAC_controller.ThermostatCurrentRemoteTemperature'+ '_' + str(i),CURRENT_OUTDOOR_TEMP + TEMP_TYPE]    
       subprocess.call(str(command),shell=True)

EDIT

OK I have rewritten this with suggestions however I still need shell=True see below

#!/usr/bin/env python

#define imports

import sys
import subprocess
import os
from subprocess import STDOUT

DEVNULL = open(os.devnull, "r+b")

#start global definitions
command = ['/Users/RPM/Applications/RacePointMedia/sclibridge servicerequest "Wine Cellar" "" "" "1" "" "Pause"']
#start main program
subprocess.call(command, close_fds=True, stdin=DEVNULL, stdout=DEVNULL, stderr=STDOUT,shell=True)

I have tried this without shell=True and I get the following error, my major concern is memory leaks.

 File "./test.py", line 14, in <module>
    subprocess.call(commamd,close_fds=True)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 493, in call
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 679, in __init__
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 1228, in _execute_child

Upvotes: 0

Views: 1372

Answers (1)

jfs
jfs

Reputation: 414079

To run subprocesses one at a time, you could use subprocess.call as @tdelaney suggested:

#..
call(command, close_fds=True,
     stdin=DEVNULL, stdout=DEVNULL, stderr=STDOUT)

You don't need shell=True, subprocess will execute sclibridge directly.

  • close_fds=True is to avoid inheriting other open file descriptors from the parent.
  • stdin, stdout, stderr are set to provide empty input and discard any output

To run subprocesses in parallel nconcurrent at a time without leaking file descriptors from subprocesses:

import os
from subprocess import STDOUT, call
from multiprocessing.dummy import Pool # use threads

DEVNULL = open(os.devnull, "r+b")
# define HVAC_ZONES, TSTAT, CURRENT_OUTDOOR_TEMP, TEMP_TYPE here
#..
file_pattern = '.HVAC_controller.ThermostatCurrentRemoteTemperature'+ '_' 
def run(zone_i):
    zone, i = zone_i
    cmd = ['/Users/RPM/Applications/RacePointMedia/sclibridge',
           'writestate', 
           zone + file_pattern + str(i),
           CURRENT_OUTDOOR_TEMP + TEMP_TYPE]
    return cmd, call(cmd, close_fds=True,
                     stdin=DEVNULL, stdout=DEVNULL, stderr=STDOUT) 

nconcurrent = 20 # limit number of concurrent processes
commands = ((zone, i+1) for zone in HVAC_ZONES for i in range(TSTAT))
pool = Pool(nconcurrent)
for cmd, returncode in pool.imap_unordered(run, commands):
    if returncode != 0:
        print("failed with returncode: %d, cmd: %s" % (returncode, cmd))
pool.close()
pool.join()
DEVNULL.close()

Upvotes: 2

Related Questions