-
Notifications
You must be signed in to change notification settings - Fork 34
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
Exit on follow by inspecting the current log line #207
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -182,12 +182,30 @@ def get_log_id(job_id): | |
return instance_log_id | ||
|
||
|
||
def check_job_termination_line(log_output_line, termination_list): | ||
""" | ||
Get Job termination by inspecting if any of the termination log lines is detected | ||
in the current log output line. | ||
""" | ||
return any(terminal_line in log_output_line for terminal_line in termination_list) | ||
|
||
|
||
def follow_logs(instance_log_id, sleep_duration=1): | ||
cur_idx = 0 | ||
while True: | ||
job_terminated = False | ||
|
||
success = "[success] Finished execution" | ||
failure = "[failed] Task execution failed" | ||
shutdown = "[shutdown] Task execution cancelled" | ||
timeout = "[timeout] Task execution cancelled" | ||
termination_list = [success, failure, shutdown, timeout] | ||
|
||
while not job_terminated: | ||
# Get the logs in a loop and log the new lines | ||
log_file_contents = ResourceClient().get_content(instance_log_id) | ||
print_output = log_file_contents[cur_idx:] | ||
# Get the status of the Job from the current log line | ||
job_terminated = check_job_termination_line(print_output, termination_list) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be simplified as:
Then you can remove the extra function call here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I cannot go for |
||
cur_idx += len(print_output) | ||
sys.stdout.write(print_output) | ||
sleep(sleep_duration) | ||
|
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.
better move this list creation outside so we are not creating a new list on every function call.
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.
Agreed, move this outside the function to the top of this file. Also since these are constants use capitalized variable names for easy identification.