Skip to content
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
1b4368b
add shellout.py skeleton
Pierre-siddall Jan 19, 2026
de73ecd
Execute a subprocess
Pierre-siddall Jan 20, 2026
f8b0a3a
Implement basic logging
Pierre-siddall Jan 20, 2026
786900f
Apply black
Pierre-siddall Jan 20, 2026
7da2a02
Add exception handling
Pierre-siddall Jan 20, 2026
da9af7d
Add timer decorator
Pierre-siddall Jan 20, 2026
bb4667a
Seperate command string using shlex
Pierre-siddall Jan 20, 2026
9ca9722
Apply black again
Pierre-siddall Jan 20, 2026
baac45d
Replace instances of common.exec_subproc
Pierre-siddall Jan 20, 2026
bf3f823
Recplace all shellout instances (AI assisted)
Pierre-siddall Jan 20, 2026
787b5b5
Apply black to all modified files
Pierre-siddall Jan 20, 2026
665b2fa
Revert "Apply black to all modified files"
Pierre-siddall Jan 20, 2026
af43b3c
Merge branch 'main' into move-shellouts
Pierre-siddall Jan 20, 2026
abe99cd
Add AI statement
Pierre-siddall Jan 20, 2026
d3aafae
Update copyright statements
Pierre-siddall Jan 20, 2026
e3219e0
Remove timer
Pierre-siddall Jan 27, 2026
b5aaefc
Add copyright notice
Pierre-siddall Jan 27, 2026
49e485d
Move shellouts to mocilib
Pierre-siddall Jan 27, 2026
4a50e2d
Deprecate old code correctly
Pierre-siddall Jan 28, 2026
ecedc2e
Turn shellout.py into a package
Pierre-siddall Jan 28, 2026
757f378
Change import statements
Pierre-siddall Jan 28, 2026
5afda7e
Revert "Add AI statement"
Pierre-siddall Jan 28, 2026
ca5b95a
Reapply "Add AI statement"
Pierre-siddall Jan 28, 2026
8c575a0
Revert "Add AI statement"
Pierre-siddall Jan 28, 2026
04e4a1f
Revert "Recplace all shellout instances (AI assisted)"
Pierre-siddall Jan 28, 2026
018bc2b
Revert "Replace instances of common.exec_subproc"
Pierre-siddall Jan 28, 2026
8074de2
Add unit test for exec_subprocess
Pierre-siddall Jan 28, 2026
19e7e24
Add first unit tests
Pierre-siddall Jan 28, 2026
4203810
Add timeout parameter to shellouts
Pierre-siddall Jan 28, 2026
17fb5c9
Add unit tests to test error conditions
Pierre-siddall Jan 28, 2026
10a063a
Fix subprocess command
Pierre-siddall Jan 28, 2026
f91f9b9
Remove stdin parameter
Pierre-siddall Jan 28, 2026
a1296d3
Remove OS test case
Pierre-siddall Jan 28, 2026
a125be9
Remove version check
Pierre-siddall Jan 28, 2026
7bb2cf1
Add subprocess.TimeoutExpired error
Pierre-siddall Jan 28, 2026
25974d2
Add timeout expired unit test
Pierre-siddall Jan 28, 2026
1191b57
Ensure completed process output is always string
Pierre-siddall Jan 29, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 21 additions & 22 deletions Coupled_Drivers/cice_driver.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env python3
'''
*****************************COPYRIGHT******************************
(C) Crown copyright 2023-2025 Met Office. All rights reserved.
(C) Crown copyright 2023-2026 Met Office. All rights reserved.

Use, duplication or disclosure of this code is subject to the restrictions
as set forth in the licence. If no licence has been raised with this copy
Expand All @@ -11,6 +11,10 @@

Met Office, FitzRoy Road, Exeter, Devon, EX1 3PB, United Kingdom
*****************************COPYRIGHT******************************

# Some of the content of this file has been produced with the assistance of
# Claude Sonnet 4.5.

NAME
cice_driver.py

Expand All @@ -30,6 +34,7 @@
import time2days
import inc_days
import common
import shellout
import error
import dr_env_lib.cice_def
import dr_env_lib.env_lib
Expand Down Expand Up @@ -184,24 +189,21 @@ def _setup_executable(common_env):

#any variables containing things that can be globbed will start with gl_
gl_step_int_match = '^dt='
_, step_int_val = common.exec_subproc(['grep', gl_step_int_match,
cice_nl])
_, step_int_val = shellout._exec_subprocess('grep %s %s' % (gl_step_int_match, cice_nl))
cice_step_int = int(re.findall(r'^dt=(\d*)\.?', step_int_val)[0])
cice_steps = (tot_runlen_sec - last_dump_seconds) // cice_step_int

_, cice_histfreq_val = common.exec_subproc(['grep', 'histfreq', cice_nl])
_, cice_histfreq_val = shellout._exec_subprocess('grep histfreq %s' % cice_nl)
cice_histfreq_val = re.findall(r'histfreq\s*=\s*(.*)', cice_histfreq_val)[0]
cice_histfreq = __expand_array(cice_histfreq_val)[1]

_, cice_histfreq_n_val = common.exec_subproc([ \
'grep', 'histfreq_n', cice_nl])
_, cice_histfreq_n_val = shellout._exec_subprocess('grep histfreq_n %s' % cice_nl)
cice_histfreq_n_val = re.findall(r'histfreq_n\s*=\s*(.*)',
cice_histfreq_n_val)[0]
cice_histfreq_n = __expand_array(cice_histfreq_n_val)
cice_histfreq_n = int(cice_histfreq_n.split(',')[0])

_, cice_age_rest_val = common.exec_subproc([ \
'grep', '^restart_age', cice_nl])
_, cice_age_rest_val = shellout._exec_subprocess('grep ^restart_age %s' % cice_nl)
cice_age_rest = re.findall(r'restart_age\s*=\s*(.*)',
cice_age_rest_val)[0]

Expand All @@ -216,7 +218,7 @@ def _setup_executable(common_env):
cice_envar['SHARED_FNAME'])
sys.exit(error.MISSING_DRIVER_FILE_ERROR)
if not common_env['MODELBASIS']:
_, modelbasis_val = common.exec_subproc('grep', 'model_basis_time',
_, modelbasis_val = shellout._exec_subprocess('grep model_basis_time %s' %
cice_envar['SHARED_FNAME'])
modelbasis_val = re.findall(r'model_basis_time\s*=\s*(.*)',
modelbasis_val)
Expand All @@ -225,7 +227,7 @@ def _setup_executable(common_env):
if not common_env['TASKSTART']:
common_env.add('TASKSTART', common_env['MODELBASIS'])
if not common_env['TASKLENGTH']:
_, tasklength_val = common.exec_subproc('grep', 'run_target_end',
_, tasklength_val = shellout._exec_subprocess('grep run_target_end %s' %
cice_envar['SHARED_FNAME'])
tasklength_val = re.findall(r'run_target_end\s*=\s*(.*)',
tasklength_val)
Expand All @@ -241,20 +243,18 @@ def _setup_executable(common_env):
// cice_step_int
else:
# This is probably a coupled NWP suite
cmd = ['rose', 'date', str(run_start[0])+'0101T0000Z',
cice_envar['TASK_START_TIME']]
_, time_since_year_start = common.exec_subproc(cmd)
cmd = 'rose date %s0101T0000Z %s' % (str(run_start[0]), cice_envar['TASK_START_TIME'])
_, time_since_year_start = shellout._exec_subprocess(cmd)
#The next command works because rose date assumes
# 19700101T0000Z is second 0
cmd = ['rose', 'date', '--print-format=%s', '19700101T00Z',
'--offset='+time_since_year_start]
cmd = 'rose date --print-format=%%s 19700101T00Z --offset=%s' % time_since_year_start
# Account for restarting from a failure in next line
# common.exec_subproc returns a tuple containing (return_code, output)
seconds_since_year_start = int(common.exec_subproc(cmd)[1]) \
# shellout._exec_subprocess returns a tuple containing (return_code, output)
seconds_since_year_start = int(shellout._exec_subprocess(cmd)[1]) \
+ last_dump_seconds
cice_istep0 = seconds_since_year_start/cice_step_int

_, cice_rst_val = common.exec_subproc(['grep', 'restart_dir', cice_nl])
_, cice_rst_val = shellout._exec_subprocess('grep restart_dir %s' % cice_nl)
cice_rst = re.findall(r'restart_dir\s*=\s*\'(.*)\',', cice_rst_val)[0]
if cice_rst[-1] == '/':
cice_rst = cice_rst[:-1]
Expand All @@ -266,9 +266,9 @@ def _setup_executable(common_env):
cice_restart = os.path.join(cice_rst,
cice_envar['CICE_RESTART'])

_, cice_hist_val = common.exec_subproc(['grep', 'history_dir', cice_nl])
_, cice_hist_val = shellout._exec_subprocess('grep history_dir %s' % cice_nl)
cice_hist = re.findall(r'history_dir\s*=\s*\'(.*)\',', cice_hist_val)[0]
_, cice_incond_val = common.exec_subproc(['grep', 'incond_dir', cice_nl])
_, cice_incond_val = shellout._exec_subprocess('grep incond_dir %s' % cice_nl)
cice_incond = re.findall(r'incond_dir\s*=\s*\'(.*)\',', cice_incond_val)[0]

for direc in (cice_rst, cice_hist, cice_incond):
Expand Down Expand Up @@ -312,8 +312,7 @@ def _setup_executable(common_env):
if cice_age_rest == 'true':
cice_runtype = 'continue'
ice_ic = 'set in pointer file'
_, _ = common.exec_subproc([cice_envar['CICE_START'],
'>', cice_restart])
_, _ = shellout._exec_subprocess('%s > %s' % (cice_envar['CICE_START'], cice_restart))
sys.stdout.write('[INFO] %s > %s' %
(cice_envar['CICE_START'],
cice_restart))
Expand Down
67 changes: 0 additions & 67 deletions Coupled_Drivers/common.py

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have definitely replace all calls to these functions from drivers code, then by all means remove these methods. rose-stem will be required to check most of these. If you have only replaced a subset then please leave these here, with "Method deprecated in favour of moci_utils.shellout" added to the docstring.

Original file line number Diff line number Diff line change
Expand Up @@ -228,73 +228,6 @@ def setup_runtime(common_env):

return runlen_sec

def exec_subproc_timeout(cmd, timeout_sec=10):
'''
Execute a given shell command with a timeout. Takes a list containing
the commands to be run, and an integer timeout_sec for how long to
wait for the command to run. Returns the return code from the process
and the standard out from the command or 'None' if the command times out.
'''
process = subprocess.Popen(cmd, shell=False,
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
timer = threading.Timer(timeout_sec, process.kill)
try:
timer.start()
stdout, err = process.communicate()
if err:
sys.stderr.write('[SUBPROCESS ERROR] %s\n' % error)
rcode = process.returncode
finally:
timer.cancel()
if sys.version_info[0] >= 3:
output = stdout.decode()
else:
output = stdout
return rcode, output


def exec_subproc(cmd, verbose=True):
'''
Execute given shell command. Takes a list containing the commands to be
run, and a logical verbose which if set to true will write the output of
the command to stdout.
'''
process = subprocess.Popen(cmd, shell=False,
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
output, err = process.communicate()
if verbose and output:
sys.stdout.write('[SUBPROCESS OUTPUT] %s\n' % output)
if err:
sys.stderr.write('[SUBPROCESS ERROR] %s\n' % error)
if sys.version_info[0] >= 3:
output = output.decode()
return process.returncode, output


def __exec_subproc_true_shell(cmd, verbose=True):
'''
Execute given shell command, with shell=True. Only use this function if
exec_subproc does not work correctly. Takes a list containing the commands
to be run, and a logical verbose which if set to true will write the
output of the command to stdout.
'''
process = subprocess.Popen(cmd, shell=True,
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
output, err = process.communicate()
if verbose and output:
sys.stdout.write('[SUBPROCESS OUTPUT] %s\n' % output)
if err:
sys.stderr.write('[SUBPROCESS ERROR] %s\n' % error)
if sys.version_info[0] >= 3:
output = output.decode()
return process.returncode, output


def _calculate_ppn_values(nproc, nodes):
'''
Expand Down
13 changes: 9 additions & 4 deletions Coupled_Drivers/cpmip_utils.py

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I think that where you have replaced common.exec_subproc_timeout(du_command, timeout) with shellout._exec_subprocess(du_command, timeout), what you have actually done is set keyword argument verbose=timeout. verbose is currently the only KW arg available currently.
Unit tests for this would make this error clear.

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env python3
'''
*****************************COPYRIGHT******************************
(C) Crown copyright 2023-2025 Met Office. All rights reserved.
(C) Crown copyright 2023-2026 Met Office. All rights reserved.

Use, duplication or disclosure of this code is subject to the restrictions
as set forth in the licence. If no licence has been raised with this copy
Expand All @@ -11,6 +11,10 @@

Met Office, FitzRoy Road, Exeter, Devon, EX1 3PB, United Kingdom
*****************************COPYRIGHT******************************

# Some of the content of this file has been produced with the assistance of
# Claude Sonnet 4.5.

NAME
cpmip_utils.py

Expand All @@ -23,6 +27,7 @@
import sys
import error
import common
import shellout

def get_component_resolution(nlist_file, resolution_variables):
'''
Expand All @@ -32,7 +37,7 @@ def get_component_resolution(nlist_file, resolution_variables):
'''
resolution = 1
for res_var in resolution_variables:
_, out = common.exec_subproc(['grep', res_var, nlist_file],
_, out = shellout._exec_subprocess('grep %s %s' % (res_var, nlist_file),
verbose=True)
try:
i_res = int(re.search(r'(\d+)', out).group(0))
Expand All @@ -56,7 +61,7 @@ def get_glob_usage(glob_path, timeout=60):
filelist = glob.glob(glob_path)
if filelist:
du_command = ['du', '-c'] + filelist
rcode, output = common.exec_subproc_timeout(du_command, timeout)
rcode, output = shellout._exec_subprocess(du_command, timeout)
if rcode == 0:
size_k = float(output.split()[-2])
else:
Expand Down Expand Up @@ -131,7 +136,7 @@ def get_workdir_netcdf_output(timeout=60):
i_f.split('.')[-1] == 'nc' and not os.path.islink(i_f)]
size_k = -1.0
du_command = ['du', '-c'] + output_files
rcode, output = common.exec_subproc_timeout(du_command, timeout)
rcode, output = shellout._exec_subprocess(du_command, timeout)
if rcode == 0:
size_k = float(output.split()[-2])
return size_k
Expand Down
11 changes: 8 additions & 3 deletions Coupled_Drivers/cpmip_xios.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env python3
'''
*****************************COPYRIGHT******************************
(C) Crown copyright 2021-2025 Met Office. All rights reserved.
(C) Crown copyright 2021-2026 Met Office. All rights reserved.

Use, duplication or disclosure of this code is subject to the restrictions
as set forth in the licence. If no licence has been raised with this copy
Expand All @@ -11,6 +11,10 @@

Met Office, FitzRoy Road, Exeter, Devon, EX1 3PB, United Kingdom
*****************************COPYRIGHT******************************

# Some of the content of this file has been produced with the assistance of
# Claude Sonnet 4.5.

NAME
cpmip_xios.py

Expand All @@ -21,6 +25,7 @@
import shutil
import sys
import common
import shellout

def data_metrics_setup_nemo():
'''
Expand Down Expand Up @@ -58,8 +63,8 @@ def measure_xios_client_times(timeout=120):
'xios_client' in i_f and 'out' in i_f]
total_files = len(files)
for i_f in files:
rcode, out = common.exec_subproc_timeout(
['grep', 'total time', i_f], timeout)
rcode, out = shellout._exec_subprocess(
'grep "total time" %s' % i_f, timeout)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you are setting verbose=timeout here - not quite what you meant?

if rcode == 0:
meas_time = float(out.split()[-2])
total_measured += 1
Expand Down
9 changes: 7 additions & 2 deletions Coupled_Drivers/mct_driver.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env python3
'''
*****************************COPYRIGHT******************************
(C) Crown copyright 2021-2025 Met Office. All rights reserved.
(C) Crown copyright 2021-2026 Met Office. All rights reserved.
Use, duplication or disclosure of this code is subject to the restrictions
as set forth in the licence. If no licence has been raised with this copy
of the code, the use, duplication or disclosure of it is strictly
Expand All @@ -10,6 +10,10 @@

Met Office, FitzRoy Road, Exeter, Devon, EX1 3PB, United Kingdom
*****************************COPYRIGHT******************************

# Some of the content of this file has been produced with the assistance of
# Claude Sonnet 4.5.

NAME
mct_driver.py

Expand All @@ -23,6 +27,7 @@
import sys
import glob
import common
import shellout
import error
import update_namcouple
import dr_env_lib.mct_def
Expand Down Expand Up @@ -238,7 +243,7 @@ def _setup_executable(common_env, envarinsts, run_info):
# Create transient field namelist (note if we're creating a
# namcouple on the fly, this will have to wait until after
# the namcouple have been created).
_, _ = common.exec_subproc('./OASIS_fields')
_, _ = shellout._exec_subprocess('./OASIS_fields')

for component in mct_envar['COUPLING_COMPONENTS'].split():
if not component in common_env['models']:
Expand Down
Loading
Loading