Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow local logs for cat-log #509

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ below.
- Mel Hall
- Christopher Bennett
- Mark Dawson
- Scott Wales
<!-- end-shortlog -->

(All contributors are identifiable with email addresses in the git version
Expand Down
9 changes: 9 additions & 0 deletions cylc/uiserver/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
from tornado import ioloop
from tornado.web import RedirectHandler
from traitlets import (
Bool,
Dict,
Float,
Int,
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -392,6 +400,7 @@ def __init__(self, *args, **kwargs):
log=self.log,
executor=self.executor,
workflows_mgr=self.workflows_mgr,
force_remote_logs=self.config.CylcUIServer.force_remote_logs,
)

def initialize_settings(self):
Expand Down
28 changes: 22 additions & 6 deletions cylc/uiserver/resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,14 @@ 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: bool = True,
file: Optional[str] = None,
):
"""Calls `cat log`.

Used for log subscriptions.
Expand All @@ -378,10 +385,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)}')
Expand Down Expand Up @@ -448,15 +456,17 @@ 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: bool):
"""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
query rather than a process held open for subscription.
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,
Expand Down Expand Up @@ -488,12 +498,14 @@ def __init__(
log: 'Logger',
workflows_mgr: 'WorkflowsManager',
executor,
force_remote_logs: bool,
**kwargs
):
super().__init__(data)
self.log = log
self.workflows_mgr = workflows_mgr
self.executor = executor
self.force_remote_logs = force_remote_logs

# Set extra attributes
for key, value in kwargs.items():
Expand Down Expand Up @@ -580,15 +592,19 @@ async def subscription_service(
ids[0],
self.log,
info,
file
file=file,
force_remote=self.force_remote_logs,
):
yield ret

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(
Expand Down
70 changes: 70 additions & 0 deletions cylc/uiserver/tests/test_resolvers.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import asyncio
from async_timeout import timeout
import io
import logging
import pytest
from unittest import mock
Expand Down Expand Up @@ -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',
[
Expand Down