diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6de00e76..618e16cd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -59,6 +59,7 @@ below. - Mel Hall - Christopher Bennett - Mark Dawson + - Scott Wales (All contributors are identifiable with email addresses in the git version diff --git a/cylc/uiserver/app.py b/cylc/uiserver/app.py index e821a6f2..0aea54dd 100644 --- a/cylc/uiserver/app.py +++ b/cylc/uiserver/app.py @@ -65,6 +65,7 @@ from tornado import ioloop from tornado.web import RedirectHandler from traitlets import ( + Bool, Dict, Float, Int, @@ -324,6 +325,13 @@ class CylcUIServer(ExtensionApp): ''', default_value=1 ) + force_remote_logs = Bool( + config=True, + help=''' + Always use --force-remote with `cat-log` + ''', + default_value=True + ) @validate('ui_build_dir') def _check_ui_build_dir_exists(self, proposed): @@ -392,6 +400,7 @@ def __init__(self, *args, **kwargs): log=self.log, executor=self.executor, workflows_mgr=self.workflows_mgr, + force_remote_logs=True, ) def initialize_settings(self): diff --git a/cylc/uiserver/resolvers.py b/cylc/uiserver/resolvers.py index e1e85e47..8ec5d2dc 100644 --- a/cylc/uiserver/resolvers.py +++ b/cylc/uiserver/resolvers.py @@ -369,7 +369,7 @@ async def enqueue(stream, queue): await queue.put(line.decode()) @classmethod - async def cat_log(cls, id_: Tokens, log, info, file=None): + async def cat_log(cls, id_: Tokens, log, info, force_remote=True, file=None): """Calls `cat log`. Used for log subscriptions. @@ -378,10 +378,11 @@ async def cat_log(cls, id_: Tokens, log, info, file=None): 'cylc', 'cat-log', '--mode=tail', - '--force-remote', '--prepend-path', id_.id, ] + if force_remote: + cmd += ['--force-remote'] if file: cmd += ['-f', file] log.info(f'$ {" ".join(cmd)}') @@ -448,7 +449,7 @@ async def cat_log(cls, id_: Tokens, log, info, file=None): yield {'connected': False} @classmethod - async def cat_log_files(cls, id_: Tokens): + async def cat_log_files(cls, id_: Tokens, force_remote): """Calls cat log to get list of available log files. Note kept separate from the cat_log method above as this is a one off @@ -456,7 +457,9 @@ async def cat_log_files(cls, id_: Tokens): This uses the Cylc cat-log interface, list dir mode, forcing remote file checking. """ - cmd: List[str] = ['cylc', 'cat-log', '-m', 'l', '-o', id_.id] + cmd: List[str] = ['cylc', 'cat-log', '-m', 'l', id_.id] + if force_remote: + cmd += ['--force-remote'] proc_job = await asyncio.subprocess.create_subprocess_exec( *cmd, stdout=asyncio.subprocess.PIPE, @@ -488,6 +491,7 @@ def __init__( log: 'Logger', workflows_mgr: 'WorkflowsManager', executor, + force_remote_logs: bool, **kwargs ): super().__init__(data) @@ -580,7 +584,8 @@ async def subscription_service( ids[0], self.log, info, - file + file, + force_remote=self.force_remote_logs, ): yield ret @@ -588,7 +593,7 @@ async def query_service( self, id_: Tokens, ): - return await Services.cat_log_files(id_) + return await Services.cat_log_files(id_, force_remote=self.force_remote_logs) def kill_process_tree( diff --git a/cylc/uiserver/tests/test_resolvers.py b/cylc/uiserver/tests/test_resolvers.py index 7ebd0993..6bc64d6c 100644 --- a/cylc/uiserver/tests/test_resolvers.py +++ b/cylc/uiserver/tests/test_resolvers.py @@ -1,5 +1,6 @@ import asyncio from async_timeout import timeout +import io import logging import pytest from unittest import mock @@ -115,6 +116,75 @@ async def test_cat_log(workflow_run_dir): assert actual.rstrip() == expected.rstrip() +async def test_cat_log_remote(workflow_run_dir): + (id_, log_dir) = workflow_run_dir + workflow = Tokens(id_) + log = logging.getLogger(CYLC_LOG) + + info = mock.MagicMock() + info.root_value = 2 + # mock the context + info.context = {'sub_statuses': {2: "start"}} + + # Mock out the `cylc cat-log` subprocess and the process killer to avoid + # side effects + with (mock.patch("asyncio.subprocess.create_subprocess_exec") as subp, + mock.patch("cylc.uiserver.resolvers.kill_process_tree") as kpt): + subp.return_value.returncode = 0 + subp.return_value.communicate = mock.AsyncMock() + subp.return_value.communicate.return_value = (b"", b"") + + async with timeout(10): + ret = services.cat_log(workflow, log, info, force_remote=False) + async for response in ret: + await asyncio.sleep(0) + + subp.assert_called_once() + assert subp.call_args[0] == ('cylc','cat-log','--mode=tail','--prepend-path', workflow.id) + subp.reset_mock() + + async with timeout(10): + ret = services.cat_log(workflow, log, info, force_remote=True) + async for response in ret: + await asyncio.sleep(0) + + subp.assert_called_once() + assert subp.call_args[0] == ('cylc','cat-log','--mode=tail','--prepend-path', workflow.id, '--force-remote') + + +async def test_cat_log_files_remote(workflow_run_dir): + (id_, log_dir) = workflow_run_dir + workflow = Tokens(id_) + log = logging.getLogger(CYLC_LOG) + + info = mock.MagicMock() + info.root_value = 2 + # mock the context + info.context = {'sub_statuses': {2: "start"}} + + # Mock out the `cylc cat-log` subprocess and the process killer to avoid + # side effects + with (mock.patch("asyncio.subprocess.create_subprocess_exec") as subp, + mock.patch("cylc.uiserver.resolvers.kill_process_tree") as kpt): + subp.return_value.returncode = 0 + subp.return_value.communicate = mock.AsyncMock() + subp.return_value.communicate.return_value = (b"", b"") + + async with timeout(10): + ret = await services.cat_log_files(workflow, force_remote=False) + + subp.assert_called_once() + assert subp.call_args[0] == ('cylc','cat-log','-m','l', workflow.id) + subp.reset_mock() + + async with timeout(10): + ret = await services.cat_log_files(workflow, force_remote=True) + + subp.assert_called_once() + assert subp.call_args[0] == ('cylc','cat-log','-m','l', workflow.id, '--force-remote') + subp.reset_mock() + + @pytest.mark.parametrize( 'text, expected', [