Reputation: 87
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
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 outputTo 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