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

PopCorn: add logging #16848

Merged
merged 1 commit into from Jan 12, 2020
Merged

PopCorn: add logging #16848

merged 1 commit into from Jan 12, 2020

Conversation

the-antz
Copy link
Contributor

@the-antz the-antz commented Nov 26, 2019

This prevents PopCorn from printing warnings to tty1 because of a (not
yet) available network connection.

@steinex
Copy link
Contributor

steinex commented Nov 28, 2019

I'd rather that PopCorn does not print the "Retrying ping" message, because it has that functionality built in already.

@the-antz
Copy link
Contributor Author

Should I change it to redirect popcorns stdout/stderr to /dev/null instead?

Or remove the message from popcorn itself?

@Piraty
Copy link
Member

Piraty commented Nov 28, 2019

My opinion: Service supervisors should not be (ab)used to fix broken daemons and their inability to handle unmet prerequisites properly (I'm looking at systemd's service dependencies for example, which people like to use to ensure other functionality – like network etc – is available before a service starts, instead of properly checking prereqs in their own code)

@the-maldridge
Copy link
Member

A log service should be added which writes the output to a file rather than to the default, which is tty1 for all runit managed services..

@the-antz
Copy link
Contributor Author

the-antz commented Nov 29, 2019

A log service should be added which writes the output to a file rather than to the default, which is tty1 for all runit managed services..

Like a --syslog or --log filename parameter for the popcorn client?

Or is something like 1>>${LOGFILE_INFO} 2>>${LOGFILE_ERR} in the runit script an acceptable way of doing it?

For now I prefix the commit message with [NOMERGE] while I dig deeper into this, I hope that's the correct way of doing it (I'm fairly newish to this).

edit:
After looking at popcorns source, it turns out "Retrying ping" is the only message that goes to stdout, all other messages go to stderr.
What do you think about changing the run script to redirect stdout to /dev/null while keeping stderr as is?

@the-antz the-antz changed the title PopCorn: make runit wait until network becomes available. [NO MERGE] PopCorn: make runit wait until network becomes available. Nov 29, 2019
@Duncaen
Copy link
Member

Duncaen commented Nov 29, 2019

Something like:

mkdir srcpkgs/PopCorn/files/popcorn/log
ln -s /usr/bin/vlogger srcpkgs/PopCorn/files/popcorn/log/run

And maybe exec 2>&1 in srcpkgs/PopCorn/files/popcorn/run if the messages end up on stderr.

Edit: this will log the messages by default to syslog with the flexibility of being able to use vlogger to redirect them to files using a /etc/vlogger script.

@the-antz the-antz changed the title [NO MERGE] PopCorn: make runit wait until network becomes available. PopCorn: add logging Nov 29, 2019
@the-antz
Copy link
Contributor Author

PR updated. The new dependency util-linux provides vlogger.

@the-antz the-antz changed the title PopCorn: add logging [NOMERGE] PopCorn: add logging Nov 29, 2019
@the-antz the-antz changed the title [NOMERGE] PopCorn: add logging PopCorn: add logging Nov 29, 2019
@Duncaen
Copy link
Member

Duncaen commented Nov 29, 2019

vlogger is part of runit-void I don't think you need to add a dependency for that, if you use the runit service you most likely have runit-void installed.

@the-antz
Copy link
Contributor Author

Fixed :)

@@ -0,0 +1 @@
/usr/bin/vlogger
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this have a #! and an exec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The file is a symlink to /usr/bin/vlogger.

This prevents PopCorn from printing warnings to tty1 because of a (not
yet) available network connection.
[skip ci]
@the-maldridge the-maldridge merged commit cc07b96 into void-linux:master Jan 12, 2020
@the-antz the-antz deleted the popcorn branch April 19, 2020 18:22
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants