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

Option to suppress the message "Starting Opium on ...." #260

Open
Nymphium opened this issue Feb 19, 2021 · 4 comments
Open

Option to suppress the message "Starting Opium on ...." #260

Nymphium opened this issue Feb 19, 2021 · 4 comments

Comments

@Nymphium
Copy link
Contributor

No description provided.

@tmattio
Copy link
Collaborator

tmattio commented Feb 20, 2021

Hi, could you describe the problem you have with the message "Starting Opium on"?

@aantron
Copy link

aantron commented Feb 27, 2021

I have my own problem with this message, which is that I'd like to replace it with a URL that will be clickable in the terminal, which I emit myself because it is based on app configuration. So I'd like to suppress this message as it is always either misleading or redundant in such an app.

Also, it, and, apparently, all the other log expressions in app.ml, print to the application logger. This is wrong because there is no good way to selectively suppress them and still retain easy logging in the true, user's application. The Opium App is, of course, still a library, not an application.

This is also the reason why Logs itself recommends against libraries logging with the default source. Opium has an opium.server source somewhere that could be used for some of these prints, but the "starting" print should be removed or at least suppressed anyway, because there are legitimate reasons why a user would want to see the other messages but not "starting."

@anuragsoni
Copy link
Collaborator

anuragsoni commented Feb 27, 2021

Also, it, and, apparently, all the other log expressions in app.ml, print to the application logger. This is wrong because there is no good way to selectively suppress them and still retain easy logging in the true, user's application. The Opium App is, of course, still a library, not an application.

Agreed. Opium shouldn't log to the default source. I'll take a look at all the places opium logs content and make sure that we only use an opium specific log source.

I think it'd be nicer if opium also removed anything related to setting up a log reporter, as it should be the application that decides which reporter should be used. It can be a little confusing if the user runs an opium app in verbose mode and opium overrides which log reporter is being used

opium/opium/src/app.ml

Lines 192 to 198 in 4e71f0b

let setup_logger app =
if app.verbose
then (
Logs.set_reporter (Logs_fmt.reporter ());
Logs.set_level (Some Logs.Info));
if app.debug then Logs.set_level (Some Logs.Debug)
;;

@Nymphium
Copy link
Contributor Author

Nymphium commented Mar 3, 2021

Others already said the reason why needed the option. It's better to control where to display or what to say any messages in server by library users.

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

No branches or pull requests

4 participants