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

Add --quiet flag to daemon and coordinator #548

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Conversation

phil-opp
Copy link
Collaborator

Suppresses all log messages to stdout.

I went with --quiet instead of changing the default and adding a --verbose arg because this even suppresses error output.

Suppresses all log messages to stdout.
@haixuanTao
Copy link
Collaborator

The thing is within the unix philosophy the default should be quiet and not verbose.

Expect the output of every program to become the input to another, as yet unknown, program. Don't clutter output with extraneous information. Avoid stringently columnar or binary input formats. Don't insist on interactive input.
https://en.wikipedia.org/wiki/Unix_philosophy

Can we make the level filter to ERROR?

@haixuanTao
Copy link
Collaborator

I think that it is fine that the error message are filtered out, if we want to have it truly quiet.

@phil-opp
Copy link
Collaborator Author

I don't think that we should follow UNIX philosophy. The daemon and coordinator are long-running, server-like programs, so nobody will use the output dora daemon as an input to another UNIX tool.

The default filter is WARN and I think that's quite suitable. I don't consider warnings as extraneous information. For example, the coordinator will log a warning if it fails to send a heartbeat message to the daemon. This indicates that something is wrong with the setup, so this isn't something that we should hide by default.

I'm fine with adjusting the log output for the daemon in a follow-up PR. For example, I don't think that we need to print dataflow-related errors and warnings by default because they are already sent back to the CLI. But this is not related to this quiet flag.

@phil-opp
Copy link
Collaborator Author

Note that a lot of UNIX commands implement a --quiet option. Examples are grep, make, or git. Some more examples are mentioned here: https://unix.stackexchange.com/questions/191254/why-do-many-commands-provide-a-quiet-option

Copy link
Collaborator

@haixuanTao haixuanTao left a comment

Choose a reason for hiding this comment

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

ok sure, you're right

@phil-opp phil-opp merged commit d04af76 into main Jun 12, 2024
18 checks passed
@phil-opp phil-opp deleted the less-verbose-daemon branch June 12, 2024 16:45
@haixuanTao
Copy link
Collaborator

haixuanTao commented Jun 13, 2024

So this merge make it that when we we use dora start without --attach flag and there is an error on initialization there is nothing that is printed out.

We need to somehow reply to dora start in case there is a failure on initialization.

@phil-opp
Copy link
Collaborator Author

So the error happens after the dataflow is considered as started an its UUID is returned?

@phil-opp
Copy link
Collaborator Author

I opened #554 to make it easier to find out whether a dataflow failed.

I don't think that we can return an error on a non-attaching dora start because it directly exits after the dataflow is started.

@haixuanTao
Copy link
Collaborator

haixuanTao commented Jun 13, 2024

Yeah we could make dora start --attach default in that case and have dora start --detached ?

It's very frustrating to not have any info on errors

@haixuanTao
Copy link
Collaborator

So the error happens after the dataflow is considered as started an its UUID is returned?

So one error is that I had a python import issue, so at the very beginning of the script before node = Node() and nothing happened.

@haixuanTao
Copy link
Collaborator

I think we should add CI/CD test on graceful stop for pending nodes.

@haixuanTao
Copy link
Collaborator

One of the problem with this PR is that when an error happens but the dataflow does not immediately end nothing happens within the terminal which is also very confusing.

If we're going #552, we need to make sure to print also errors that happens before the entire dataflow finishes.

@phil-opp
Copy link
Collaborator Author

Good idea! I try to implement this.

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

Successfully merging this pull request may close these issues.

None yet

2 participants