-
Notifications
You must be signed in to change notification settings - Fork 94
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: view retrieved remote logs locally. #2624
cat-log: view retrieved remote logs locally. #2624
Conversation
0c13640
to
77b120a
Compare
77b120a
to
43c5d2c
Compare
tests added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test good in my environment. Some minor comments.
bin/cylc-cat-log
Outdated
Print, edit, or tail-follow content, or print path or list directory, of local | ||
or remote task job logs and suite server logs. Batch-system-specific job cat or | ||
tail commands are used if defined in global config. | ||
Cat (print), edit, or tail-follow content, or print path or list directory, of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Print is actually more correct here.
The original Unix cat command does concatenate files and print on the standard output. Correct me if I am wrong, but I don't think this command actually does concatenate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think unix cat
is almost universally used to print a single file in practice - but fair point, you're right about it's origins and functionality. I'll correct the command help ... but this does suggest a bigger problem in that our command name is cylc cat-log
!
# Cat the remote one. | ||
TEST_NAME=${TEST_NAME_BASE}-task-out | ||
run_ok $TEST_NAME cylc cat-log -f o $SUITE_NAME a-task.1 | ||
grep_ok '^the quick brown fox$' ${TEST_NAME}.stdout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to run purge_suite
to tidy up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
# Cat the local one. | ||
TEST_NAME=${TEST_NAME_BASE}-out-loc | ||
run_ok $TEST_NAME cylc cat-log -f o $SUITE_NAME a-task.1 | ||
grep_ok '^the quick brown FOX$' ${TEST_NAME}.stdout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to run purge_suite
to tidy up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sensible & logical change with suitable tests which pass locally.
A small follow-up to #2503, to avoid unnecessary ssh connections. Current cat-log tests all pass on this branch with log retrieval or not configured in
global-tests.rc
, but before requesting review I'll add another test to distinguish the two cases.