cat-log tail-end mode - #7414
Conversation
I think your new way is fine as this is an interactive thing, and if users don't realize the semantics have changed they'll still get the end of the file, which is likely what's wanted most of the time anyway. |
|
Trying to recall why we need a configurable tail command template. Is it because you (MO) use a custom command to tail job logs in the PBS staging area? Here, we tell PBS to write directly to the final log destination. |
… the middle of the log. Also added some markers to the stream so uiserver and ui know where to truncate/pop old lines.
|
I think the default |
…ed arguments to preserve old behaviour.
| platform = { | ||
| 'tail command template': 'tail -n +1 --follow=name %(filename)s', | ||
| 'tail from end command template': | ||
| 'tail -n %(lines)s --follow=name %(filename)s', | ||
| } |
There was a problem hiding this comment.
This platform dict looks identical in each test - could be extracted into a class variable?
Or, instead of using a test class, what we would tend to do for this is a @pytest.mark.parametrized test
There was a problem hiding this comment.
Ok makes sense thanks, I'll try that.
There was a problem hiding this comment.
I think i've done that properly this time.
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
ChrisPaulBennett
left a comment
ChrisPaulBennett
left a comment
There was a problem hiding this comment.
Looks good. Could we bump up the codecov a bit?
| captured['kwargs'] = kwargs | ||
| return 0 | ||
|
|
||
| monkeypatch.setattr( |
There was a problem hiding this comment.
| monkeypatch.setattr( | |
| lambda *a, **k: '/remote/workflow/log/job.out', |
There was a problem hiding this comment.
I think the suggestion is messed up due to latter commits messing with line numbers? I think this was the intent which I have applied:
async def mock_remote_cylc_cmd(cmd, platform, **kwargs):
captured['cmd'] = cmd
captured['kwargs'] = kwargs
return 0
monkeypatch.setattr(
'cylc.flow.scripts.cat_log.remote_cylc_cmd',
mock_remote_cylc_cmd
)
monkeypatch.setattr(
'cylc.flow.scripts.cat_log.get_remote_workflow_run_job_dir',
lambda *a, **k: '/remote/workflow/log/job.out',
)
monkeypatch.setattr(
'cylc.flow.scripts.cat_log.verbosity_to_opts',
lambda *a, **k: []
)
Mmm, this shouldn't make a difference |
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
Oh, maybe my tests just dont cover what I think they do then :D |
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
|
I can't seem to patch this gap: |
|
@samuel-denton I'm pretty sure this is a long-standing bug in Codecov that they never fixed: codecov/feedback#556. For example, there are lines that show coverage hits despite being non-executable (e.g. blank lines or comments)! TLDR; try merging master into your branch, or rebasing onto master |
Ok that seems likely. Ill save the rebase until I am back from holiday as I don't want to rush it. |
4 participants
Trio of PRs to add a new tail mode to cat-log and make use of it in the GUI
The three PR's will close
Changes will be documented in cylc-doc:
Added a new mode to cat-log called tail and renamed the old mode to tail_from_start.
Behaviour of the old mode should be identical.
The new
tail_from_endmode takes a line_count param as it needs to know where to start tailing from, and can follow as lines are added.Check List
CONTRIBUTING.mdand added my name as a Code Contributor.setup.cfg(andconda-environment.ymlif present).?.?.xbranch.