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 [p]set logging to Core #6172

Open
wants to merge 1 commit into
base: V3/develop
Choose a base branch
from

Conversation

Drapersniper
Copy link
Contributor

Description of the changes

As discussed with @TrustyJAID

Have the changes in this PR been tested?

No

@github-actions github-actions bot added the Category: Core - Bot Commands This is related to core commands (Core and CogManagerUI cog classes). label May 23, 2023
@Jackenmen
Copy link
Member

I don't know how I feel about this but I have a couple of implementation suggestions:

  • I would use level names (case-insensitive) rather than numbers, most of us know the level names while I doubt many would be able to give the level number on the spot without doing the math (or looking at help) first
  • The help docstring should mention that this is not a persistent setting and the values will be reset on restart

@Jackenmen Jackenmen added Type: Feature New feature or request. Status: Needs Discussion Needs more discussion. labels Jun 20, 2023
@TrustyJAID
Copy link
Member

I would like to see this added because it simplifies my setup and usage of logging. Originally this started because I was using debug for almost all logging on my cogs. Some of those logs ended up being rather spammy and in some cases unnecessary since the reason for using the debug log was for building and testing things. As a result draper was using one of my cogs on his bot testing some other stuff and was getting a bunch of logs unrelated to something he was working on.

I had not transitioned to the new verbose and trace logging and felt it wasn't worthwhile. I knew they existed but why would I bother using them when debug was often good enough? If I had a command like this in core I would be more open to using trace and verbose logging. For example if I am trying to figure out an obscure bug, normally I would just slap debug logs deeper and deeper until I figure it out. Often times I'll forget to remove those debug logs that may not contain anything relevant. If I'm able switch between logging levels while digging I can more easily use the new trace and verbose and not worry about spamming people with now useless logs.

Restarting the bot isn't always the easiest solution and can break up the flow when writing code. If I was running my bot with debug enabled and then decided to add a trace line and see what's happening. Right now I would have to restart my bot and in those few seconds, where I take all attention off my code to restart the bot and wait for it to connect, I might lose my train of thought. The same can be said in reverse, if I was in trace logging I might find it difficult to track down a new debug line I added and would have to restart the bot just to cleanly see what's going on.

Another potential benefit in having this is being able to ask users to switch to debug logging temporarily to get more info on an issue. From a quick glance this doesn't seem beneficial for anything in core but it has some potential benefits for cog creators. Asking a user to eval this change is bad practice and having a command from core that cog creators can suggest is a lot better.

This is a personal reason for wanting this command but still somewhat relevant. I run my dev bot though systemd and modifying the startup to change logging level isn't the easiest. I know that running my dev bot through systemd isn't necessarily recommended but it happened out of necessity and I've just been working with it since. I also don't see any particular reason one couldn't or shouldn't run a development bot through systemd. It provides some benefits such as having automatic restart.

I am fine with this not being persistent. The ease of use as a command is more than enough of a quality of life for writing cogs. Adding any sort of persistence would just complicate things and there's are better alternatives if you want to adjust the recommended default settings.

I would also like to see the level names instead of numbers as Jack suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Core - Bot Commands This is related to core commands (Core and CogManagerUI cog classes). Status: Needs Discussion Needs more discussion. Type: Feature New feature or request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants