diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 44346cac..84a448fd 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -13,7 +13,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ["3.10", "3.11"] + python-version: ["3.11"] steps: - uses: actions/checkout@v3 @@ -79,7 +79,7 @@ jobs: - name: Set up Python uses: actions/setup-python@v4 with: - python-version: "3.10" + python-version: "3.11" - name: Install dependencies run: | diff --git a/.github/workflows/build_docs.yaml b/.github/workflows/build_docs.yaml index 8d5f378e..ce6f9203 100644 --- a/.github/workflows/build_docs.yaml +++ b/.github/workflows/build_docs.yaml @@ -18,7 +18,7 @@ jobs: - name: Set up Python uses: actions/setup-python@v3 with: - python-version: '3.10' + python-version: '3.11' cache: "pip" cache-dependency-path: "setup.cfg" @@ -35,7 +35,7 @@ jobs: run: pip install --no-deps -v . - name: Install documenteer - run: pip install 'documenteer[pipelines]<0.8' + run: pip install 'documenteer[pipelines]>=0.8' - name: Build documentation working-directory: ./doc diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 1d172e0f..c2587f54 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,13 +1,13 @@ repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.4.0 + rev: v4.5.0 hooks: - id: check-yaml - id: end-of-file-fixer - id: trailing-whitespace - id: check-toml - repo: https://github.com/psf/black-pre-commit-mirror - rev: 23.9.1 + rev: 23.11.0 hooks: - id: black # It is recommended to specify the latest version of Python @@ -22,6 +22,6 @@ repos: name: isort (python) - repo: https://github.com/astral-sh/ruff-pre-commit # Ruff version. - rev: v0.0.289 + rev: v0.1.6 hooks: - id: ruff diff --git a/doc/changes/DM-41832.removal.md b/doc/changes/DM-41832.removal.md new file mode 100644 index 00000000..6c45f834 --- /dev/null +++ b/doc/changes/DM-41832.removal.md @@ -0,0 +1,3 @@ +Support for fork option in `pipetask run` has been removed as unsafe. +Default start option now is `spawn`, `forkserver` is also available. +The `fork` option is still present in CLI for compatibility, but is deprecated and replaced by `spawn` if specified. diff --git a/python/lsst/ctrl/mpexec/cli/opt/options.py b/python/lsst/ctrl/mpexec/cli/opt/options.py index 05c200da..fc6464dd 100644 --- a/python/lsst/ctrl/mpexec/cli/opt/options.py +++ b/python/lsst/ctrl/mpexec/cli/opt/options.py @@ -429,7 +429,10 @@ "--start-method", default=None, type=click.Choice(choices=["spawn", "fork", "forkserver"]), - help="Multiprocessing start method, default is platform-specific.", + help=( + "Multiprocessing start method, default is platform-specific. " + "Fork method is no longer supported, spawn is used instead if fork is selected." + ), ) diff --git a/python/lsst/ctrl/mpexec/cli/script/run.py b/python/lsst/ctrl/mpexec/cli/script/run.py index 44d48a95..9cc0dc50 100644 --- a/python/lsst/ctrl/mpexec/cli/script/run.py +++ b/python/lsst/ctrl/mpexec/cli/script/run.py @@ -182,6 +182,11 @@ def run( # type: ignore function and pass all the option kwargs to each of the script functions which ignore these unused kwargs. """ + # Fork option still exists for compatibility but we use spawn instead. + if start_method == "fork": + start_method = "spawn" + _log.warning("Option --start-method=fork is unsafe and no longer supported, will use spawn instead.") + args = SimpleNamespace( pdb=pdb, graph_fixup=graph_fixup, diff --git a/python/lsst/ctrl/mpexec/cli/script/run_qbb.py b/python/lsst/ctrl/mpexec/cli/script/run_qbb.py index 378e84d8..72184d63 100644 --- a/python/lsst/ctrl/mpexec/cli/script/run_qbb.py +++ b/python/lsst/ctrl/mpexec/cli/script/run_qbb.py @@ -25,10 +25,13 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +import logging from types import SimpleNamespace from ... import CmdLineFwk, TaskFactory +_log = logging.getLogger(__name__) + def run_qbb( butler_config: str, @@ -97,6 +100,11 @@ def run_qbb( implies no limit. The string can be either a single integer (implying units of MB) or a combination of number and unit. """ + # Fork option still exists for compatibility but we use spawn instead. + if start_method == "fork": + start_method = "spawn" + _log.warning("Option --start-method=fork is unsafe and no longer supported, will use spawn instead.") + args = SimpleNamespace( butler_config=butler_config, qgraph=qgraph, diff --git a/python/lsst/ctrl/mpexec/mpGraphExecutor.py b/python/lsst/ctrl/mpexec/mpGraphExecutor.py index 7020db50..51aa08ca 100644 --- a/python/lsst/ctrl/mpexec/mpGraphExecutor.py +++ b/python/lsst/ctrl/mpexec/mpGraphExecutor.py @@ -100,7 +100,7 @@ def terminated(self) -> bool: def start( self, quantumExecutor: QuantumExecutor, - startMethod: Literal["spawn"] | Literal["fork"] | Literal["forkserver"] | None = None, + startMethod: Literal["spawn"] | Literal["forkserver"], ) -> None: """Start process which runs the task. @@ -119,11 +119,13 @@ def start( logConfigState = CliLog.configState mp_ctx = multiprocessing.get_context(startMethod) - self.process = mp_ctx.Process( + self.process = mp_ctx.Process( # type: ignore[attr-defined] target=_Job._executeJob, args=(quantumExecutor, taskDef, quantum_pickle, logConfigState, snd_conn), name=f"task-{self.qnode.quantum.dataId}", ) + # mypy is getting confused by multiprocessing. + assert self.process is not None self.process.start() self.started = time.time() self._state = JobState.RUNNING @@ -268,7 +270,7 @@ def submit( self, job: _Job, quantumExecutor: QuantumExecutor, - startMethod: Literal["spawn"] | Literal["fork"] | Literal["forkserver"] | None = None, + startMethod: Literal["spawn"] | Literal["forkserver"], ) -> None: """Submit one more job for execution @@ -380,7 +382,7 @@ def __init__( timeout: float, quantumExecutor: QuantumExecutor, *, - startMethod: Literal["spawn"] | Literal["fork"] | Literal["forkserver"] | None = None, + startMethod: Literal["spawn"] | Literal["forkserver"] | None = None, failFast: bool = False, pdb: str | None = None, executionGraphFixup: ExecutionGraphFixup | None = None, @@ -393,11 +395,9 @@ def __init__( self.executionGraphFixup = executionGraphFixup self.report: Report | None = None - # We set default start method as spawn for MacOS and fork for Linux; - # None for all other platforms to use multiprocessing default. + # We set default start method as spawn for all platforms. if startMethod is None: - methods = dict(linux="fork", darwin="spawn") - startMethod = methods.get(sys.platform) # type: ignore + startMethod = "spawn" self.startMethod = startMethod def execute(self, graph: QuantumGraph) -> None: