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

cat-log: fix file listing errors #5514

Merged
merged 1 commit into from
May 4, 2023
Merged
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
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ ones in. -->

### Fixes

[#5514](https://github.com/cylc/cylc-flow/pull/5514) -
Ensure `cylc cat-log` directory listings always include the `job-activity.log`
file when present and are able to list submit-failed jobs.

[#5506](https://github.com/cylc/cylc-flow/pull/5506) -
Fix bug introduced in 8.1.3 where specifying a subshell command for
`flow.cylc[runtime][<namespace>][remote]host` (e.g. `$(rose host-select)`)
Expand Down
59 changes: 45 additions & 14 deletions cylc/flow/scripts/cat_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,18 +261,18 @@ def view_log(
# Print location even if the workflow does not exist yet.
print(logpath)
return 0
if not os.path.exists(logpath) and batchview_cmd is None:
Copy link
Member Author

Choose a reason for hiding this comment

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

The logpath contains the filename (defaults to job.out) even if it was not specified, even if we are listing a directory.

Consequently this caused failures when listing remote directories where the job.out file was not present.

# Note: batchview_cmd may not need to have access to logpath, so don't
# test for existence of path if it is set.
sys.stderr.write('file not found: %s\n' % logpath)
return 1
if mode == 'print-dir':
print(os.path.dirname(logpath))
return 0
if mode == 'list-dir':
for entry in sorted(os.listdir(os.path.dirname(logpath))):
print(entry)
return 0
if not os.path.exists(logpath) and batchview_cmd is None:
# Note: batchview_cmd may not need to have access to logpath, so don't
# test for existence of path if it is set.
sys.stderr.write('file not found: %s\n' % logpath)
return 1
if prepend_path:
from cylc.flow.hostuserutil import get_host
print(f'# {get_host()}:{logpath}')
Expand Down Expand Up @@ -364,17 +364,21 @@ def get_option_parser() -> COP:


def get_task_job_attrs(workflow_id, point, task, submit_num):
"""Return job (platform, job_runner_name, live_job_id).
"""Retrieve job info from the database.

* live_job_id is the job ID if job is running, else None.
* submit_failed is True if the the submission failed.

live_job_id is the job ID if job is running, else None.
Returns:
tuple - (platform, job_runner_name, live_job_id, submit_failed)

"""
with CylcWorkflowDAO(
get_workflow_run_pub_db_path(workflow_id), is_public=True
) as dao:
task_job_data = dao.select_task_job(point, task, submit_num)
if task_job_data is None:
return (None, None, None)
return (None, None, None, None)
job_runner_name = task_job_data["job_runner_name"]
job_id = task_job_data["job_id"]
if (not job_runner_name or not job_id
Expand All @@ -383,7 +387,12 @@ def get_task_job_attrs(workflow_id, point, task, submit_num):
live_job_id = None
else:
live_job_id = job_id
return (task_job_data["platform_name"], job_runner_name, live_job_id)
return (
task_job_data["platform_name"],
job_runner_name,
live_job_id,
bool(task_job_data['submit_status']),
)


@cli_function(get_option_parser)
Expand Down Expand Up @@ -512,7 +521,7 @@ def main(
with suppress(KeyError):
options.filename = JOB_LOG_OPTS[options.filename]
# KeyError: Is already long form (standard log, or custom).
platform_name, job_runner_name, live_job_id = get_task_job_attrs(
platform_name, _, live_job_id, submit_failed = get_task_job_attrs(
workflow_id, point, task, submit_num)
platform = get_platform(platform_name)
batchview_cmd = None
Expand All @@ -538,11 +547,24 @@ def main(
batchview_cmd = batchview_cmd_tmpl % {
"job_id": str(live_job_id)}

local_log_dir = get_workflow_run_job_dir(
workflow_id, point, task, submit_num
)

log_is_remote = (is_remote_platform(platform)
and (options.filename != JOB_LOG_ACTIVITY))
log_is_retrieved = (platform['retrieve job logs']
and live_job_id is None)
if log_is_remote and (not log_is_retrieved or options.force_remote):
if (
# only go remote for log files we can't get locally
log_is_remote
# don't look for remote log files for submit-failed tasks
# (there might not be any at all)
and not submit_failed
# don't go remote if the log should be retrieved (unless
# --force-remote is specified)
and (not log_is_retrieved or options.force_remote)
):
logpath = os.path.normpath(get_remote_workflow_run_job_dir(
workflow_id, point, task, submit_num,
options.filename))
Expand All @@ -568,11 +590,20 @@ def main(
manage=(mode == 'tail'),
text=False
)
if (
mode == 'list-dir'
and os.path.exists(
os.path.join(
local_log_dir,
'job-activity.log'
)
)
):
# add the local-only job-activity.log file to the remote-list
print('job-activity.log')
else:
# Local task job or local job log.
logpath = os.path.normpath(get_workflow_run_job_dir(
workflow_id, point, task, submit_num,
options.filename))
logpath = os.path.join(local_log_dir, options.filename)
tail_tmpl = os.path.expandvars(platform["tail command template"])
out = view_log(
logpath,
Expand Down
9 changes: 8 additions & 1 deletion tests/functional/cylc-cat-log/00-local.t
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
# Test "cylc cat-log" on the workflow host.
. "$(dirname "$0")/test_header"
#-------------------------------------------------------------------------------
set_test_number 41
set_test_number 43
install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"
#-------------------------------------------------------------------------------
TEST_NAME="${TEST_NAME_BASE}-validate"
Expand Down Expand Up @@ -142,6 +142,13 @@ run_ok "${TEST_NAME}-get-path" cylc cat-log -m p "${WORKFLOW_NAME}//1/a-task"
run_ok "${TEST_NAME}" cylc cat-log --prepend-path "${WORKFLOW_NAME}//1/a-task"
grep_ok "$(cat "#.*${TEST_NAME}-get-path.stdout")" "${TEST_NAME}.stdout"
#-------------------------------------------------------------------------------
TEST_NAME=${TEST_NAME_BASE}-submit-failed
run_ok "${TEST_NAME}" cylc cat-log -m l "${WORKFLOW_NAME}//1/submit-failed"
contains_ok "${TEST_NAME}.stdout" <<__END__
job.tmp
job-activity.log
__END__
#-------------------------------------------------------------------------------
TEST_NAME=${TEST_NAME_BASE}-list-no-install-dir
rm -r "${WORKFLOW_RUN_DIR}/log/install"
run_ok "${TEST_NAME}-get-path" cylc cat-log -m l "${WORKFLOW_NAME}"
Expand Down
8 changes: 7 additions & 1 deletion tests/functional/cylc-cat-log/00-local/flow.cylc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
inactivity timeout = PT3M
[scheduling]
[[graph]]
R1 = a-task
R1 = submit-failed:submit-failed => a-task
[runtime]
[[a-task]]
script = """
Expand All @@ -18,4 +18,10 @@
echo "drugs and money" > ${CYLC_TASK_LOG_ROOT}.custom-log
# Generate a warning message in the workflow log.
cylc message -p WARNING 'marmite and squashed bananas'
# Remove the submit-failed task.
cylc remove "${CYLC_WORKFLOW_NAME}//1/submit-failed"
"""
[[submit-failed]]
# This causes a submission failure due to Bash syntax check.
# In this case the job file isn't even written remotely.
script = if
1 change: 1 addition & 0 deletions tests/functional/cylc-cat-log/01-remote.t
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ TEST_NAME=${TEST_NAME_BASE}-task-list-remote-NN
cylc cat-log -f j -m l "${WORKFLOW_NAME}//1/a-task" >"${TEST_NAME}.out"
contains_ok "${TEST_NAME}.out" <<__END__
job
job-activity.log
job.custom-log
job.err
job.out
Expand Down