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 jobs to choose when to save log output #1082

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Sumukh
Copy link
Member

@Sumukh Sumukh commented Feb 19, 2017

If a message is logged with a level of critical (or higher) - the log is immediately saved (instead of having to wait for 10 more messages)

This allows jobs some level of control over what gets displayed when. It's especially useful for jobs that might not print lots of information (like the autograding job - previously the user had to wait for about a minute before they got any output)

Copy link
Contributor

@knrafto knrafto left a comment

Choose a reason for hiding this comment

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

I don't like using CRITICAL for info logging - it should be reserved for unrecoverable errors. It's also not obvious or documented, so job writers aren't likely to use it.

How about instead, if the logging handler receives a message and it has been more than X seconds since the last flush, then flush the log output anyway? It would allow slow-printing jobs to show output more quickly without requiring special attention from the job writer.

@Sumukh
Copy link
Member Author

Sumukh commented Feb 19, 2017

Sure - that should work. What do you think a reasonable interval is? 15?

@knrafto
Copy link
Contributor

knrafto commented Feb 19, 2017

Sure, it might require some manual tuning to find the right number

@colinschoen
Copy link
Member

@Sumukh Are you still planning on working on this PR?

@Sumukh
Copy link
Member Author

Sumukh commented May 1, 2018

@colinschoen Nope. Feel free to take this over.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants