Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
5 changes: 3 additions & 2 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 @@ -26,14 +26,15 @@
import re
import datetime
import time
import subprocess
import time2days
import inc_days
import common
import error
import dr_env_lib.cice_def
import dr_env_lib.env_lib

from mocilib import shellout


def __expand_array(short_array):
'''
Expand Down
76 changes: 76 additions & 0 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 @@ -296,6 +296,82 @@ def __exec_subproc_true_shell(cmd, verbose=True):
return process.returncode, output


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.

This function is now DEPRECATED in favour of the exec_subprocess function
used in mocilib/shellout.py
'''
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.

This function is now DEPRECATED in favour of the exec_subprocess function
used in mocilib/shellout.py
'''
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.

This function is now DEPRECATED in favour of the exec_subprocess function
used in mocilib/shellout.py
'''
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):
'''
Calculates number of processes per node and numa node for launch
Expand Down
2 changes: 1 addition & 1 deletion 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 Down
2 changes: 1 addition & 1 deletion 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 Down
5 changes: 4 additions & 1 deletion 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 @@ -28,6 +28,9 @@
import dr_env_lib.mct_def
import dr_env_lib.env_lib
import cpmip_controller

from mocilib import shellout

try:
import f90nml
except ImportError:
Expand Down
4 changes: 3 additions & 1 deletion Coupled_Drivers/nemo_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 Down Expand Up @@ -29,6 +29,8 @@
import common
import error

from mocilib import shellout

try:
import cf_units
except ImportError:
Expand Down
4 changes: 3 additions & 1 deletion Coupled_Drivers/rivers_driver.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env python3
'''
*****************************COPYRIGHT******************************
(C) Crown copyright 2025 Met Office. All rights reserved.
(C) Crown copyright 2025-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 @@ -26,6 +26,8 @@
import error
import dr_env_lib.rivers_def
import dr_env_lib.env_lib

from mocilib import shellout
try:
import f90nml
except ImportError:
Expand Down
4 changes: 3 additions & 1 deletion Coupled_Drivers/si3_controller.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 @@ -26,6 +26,8 @@
import dr_env_lib.ocn_cont_def
import dr_env_lib.env_lib

from mocilib import shellout

def _check_si3nl_envar(envar_container):
'''
Get the si3 namelist file exists
Expand Down
4 changes: 3 additions & 1 deletion Coupled_Drivers/top_controller.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 Down Expand Up @@ -73,6 +73,8 @@
import dr_env_lib.ocn_cont_def
import dr_env_lib.env_lib

from mocilib import shellout

# Define errors for the TOP controller only
SERIAL_MODE_ERROR = 99

Expand Down
2 changes: 1 addition & 1 deletion Coupled_Drivers/unittests/test_cpmip_utils.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 Down
2 changes: 1 addition & 1 deletion Coupled_Drivers/unittests/test_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 Down
2 changes: 1 addition & 1 deletion Coupled_Drivers/unittests/test_rivers_driver.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env python3
'''
*****************************COPYRIGHT******************************
(C) Crown copyright 2025 Met Office. All rights reserved.
(C) Crown copyright 2025-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 Down
4 changes: 3 additions & 1 deletion Coupled_Drivers/write_namcouple.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 @@ -25,6 +25,8 @@
import write_namcouple_fields
import write_namcouple_header

from mocilib import shellout

# Dictionary containing the RMP mappings
RMP_MAPPING = {'Bc':'BICUBIC',
'Bi':'BILINEA',
Expand Down
4 changes: 3 additions & 1 deletion Coupled_Drivers/xios_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 @@ -27,6 +27,8 @@
import dr_env_lib.xios_def
import dr_env_lib.env_lib

from mocilib import shellout

def _copy_iodef_custom(xios_evar):
'''
If a custom iodef file exists, copy this to the required input filename
Expand Down
2 changes: 2 additions & 0 deletions Postprocessing/common/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import subprocess
import timer

from mocilib import shellout


globals()['debug_mode'] = None
globals()['debug_ok'] = True
Expand Down
33 changes: 33 additions & 0 deletions Postprocessing/unittests/test_shellout.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# -----------------------------------------------------------------------------
# (C) Crown copyright Met Office. All rights reserved.
# The file LICENCE, distributed with this code, contains details of the terms
# under which the code may be used.
# -----------------------------------------------------------------------------

import unittest
from mocilib import shellout
from hypothesis import given, strategies as st

class ExecTets(unittest.TestCase):
''' Unit tests for executing shellout commands'''

def test_semicolon_commands(self):
cmd = "echo Hello There;echo General Kenobi"
_,rcode = shellout._exec_subprocess(cmd=cmd)
assert rcode == 0

def test_and_commands(self):
cmd ="echo Hello There&&echo General Kenobi"
_,rcode = shellout._exec_subprocess(cmd=cmd)
assert rcode == 0

@given(st.text())
def test_called_process_error(self,directory):
cmd = f"ls /{directory}"
_,rcode = shellout._exec_subprocess(cmd=cmd)
assert rcode != 0

def test_timeout_expired(self):
cmd = "sleep 15"
_,rcode = shellout._exec_subprocess(cmd=cmd,timeout=10)
assert rcode != 0
2 changes: 2 additions & 0 deletions mocilib/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
___all__ = ["shellout"]
from . import shellout
51 changes: 51 additions & 0 deletions mocilib/shellout.py

Choose a reason for hiding this comment

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

A couple of things to note here:

  • Copyright notice required. Please use the appropriate (Python) Momentum template:
    https://github.com/MetOffice/Momentum/blob/main/docs/COPYRIGHT_TEMPLATE
  • Unit tests required for this code. MOCI codebase test coverage currently uses unittest, mainly due to the age of the code. pytest may also suitable.
  • Don't use timer here - this is PostProc functionality - leave it to the PostProc application to run any timing deemed necessary. As an aside - moving timer.py into the new mocilib package is a good idea.
  • I think you've made a good decision to use subprocess.run() here, to replace the driver use of popen() and PP use of check_output(). However, a couple of things to note about the implementation:
    • L26 - I'm not convinced subprocess.stdin=True KW argument is required here. I believe that it is implied in internal calls to subprocess.communicate(), which we are not calling directly.
    • L29 - Hard-wiring timeout=10 is going to be a problem of for shell-outs which take minutes rather than seconds to complete - such as where it is required for means creation and compression of NetCDF files in PostProc. Perhaps an additional keyword argument to the routine at L9 would be useful - timeout=None?
    • L30 - Consider the use of the KW argument run(check=True) - I cannot think of a reason why we might NOT want to check the output? This saves the if statement at L35, since a subprocess.CalledProcessError exception would be raised if rcode != 0
    • L37 - The run(check_output) argument was added at Python3.7. This renders the version check here redundant. Policy, going forward, is not to support Python2 anyway. With this in mind - we may need to enforce Python3.7+ at package top level.
    • L40 - Consider catching the exception subprocess.TimeoutExpired
    • L43 - What is the use-case for the OSError exception being raised? Is this a copy and paste from PostProc, because if so, I'm not sure it is relevant for this implementation. Please use unit tests to ensure all code is executable as designed.
    • L47 - Please ensure that the return value types are (int, str) in all cases - In normal execution, output is currently an instance of subprocess.completedprocess. At L38, we need output = output.stdout.decode(). Possibly rename the return value output to avoid confusion.

Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# -----------------------------------------------------------------------------
# (C) Crown copyright Met Office. All rights reserved.
# The file LICENCE, distributed with this code, contains details of the terms
# under which the code may be used.
# -----------------------------------------------------------------------------

import subprocess
import os
import sys
import shlex


def _exec_subprocess(cmd, verbose=False, timeout=None ,current_working_directory=os.getcwd()):
"""
Execute a given shell command

:param cmd: The command to be executed given as a string
:param verbose: A boolean value to determine if the stdout
stream is displayed during the runtime.
:param current_working_directory: The directory in which the
command should be executed.
"""

cmd = shlex.split(cmd)

try:

output = subprocess.run(
cmd,
capture_output=True,
cwd=current_working_directory,
timeout=timeout,
check=True
)
rcode = output.returncode
output_message = output.stdout.decode()

if verbose and output:
sys.stdout.write(f"[DEBUG]{output.stdout}\n")
if output.stderr and output.returncode != 0:
sys.stderr.write(f"[ERROR] {output.stderr}\n")

except subprocess.CalledProcessError as exc:
output_message = exc.stdout.decode() if exc.stdout else ""
rcode = exc.returncode

except subprocess.TimeoutExpired as exc:
output_message = exc.stdout.decode() if exc.stdout else ""
rcode = exc.returncode

return rcode,output_message