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

Don't log ipv6 warning #29

Closed
wants to merge 1 commit into from
Closed

Conversation

JoNil
Copy link

@JoNil JoNil commented Apr 21, 2021

In my application i get this warning on startup, we expose the log to our user and it is not acceptable for us to spam the user with warnings.

If you don't want to accept this as is there are two other alternatives that i see. We could add a verbose flag that logs this warning or we could add an option to only start in ipv4 mode and then this could be a hard error if you ask for ipv6.

I could implement one of those options if you would like.

@willstott101
Copy link
Contributor

My intention with many of the log lines is that dependent executables could use the available metadata to filter log messages to what they deem suitable (librespot does this) - as it's unlikely we could ever please everybody. However... complaints about log verbosity keep coming up - so I wonder if it'd be best to just have a compile-time feature to completely disable logging or something.

Regardless I'd be willing to downgrade this message from warn to info if that might work for you - seems a shame to throw away the error message completely.

@ashthespy
Copy link
Member

On a tangent, turning off ipv6 in such a case might help? (#8)

@willstott101 willstott101 deleted the branch librespot-org:master May 8, 2022 14:28
@willstott101
Copy link
Contributor

Closing this as there are several other solutions. If you care deeply about the log messages you show - I think you filtering in your app might be a good idea.

Even so, a new PR to add an option to disable IPv6 would be accepted, as would a PR to reduce the log level of this particular log line. Thanks

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.

3 participants