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

WIP: Add configurable logger feature #467

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

matthiasbeyer
Copy link
Contributor

Hi,

I am currently playing with cursive and had the problem that I cannot configure the logger, so I had a look at the codebase and found that there is just no configurable logger yet.

So this is my proposal (I am planning to implement more features for the logger, most notably handlebars-based templating if you don't mind).

If you approve, we can discuss how to continue on this idea.
I made it available under a feature flag because more features might pull in quite a lot of dependencies (handlebars for example), so I figured that might be a good way.

Of course, this is WIP and not final.

Signed-off-by: Matthias Beyer <[email protected]>
@gyscos
Copy link
Owner

gyscos commented Jul 10, 2020

Thanks for the PR!

I'm not 100% sure yet where I want to go with the logger - it started as an experiment to have some debugging, but then cursive-flexi-logger-view appeared, and is probably better in every way.

The question is then: what is worth maintaining in cursive itself. I don't think there are many things that can only be done in cursive, so whatever we have here will just be one competitor among others (much like the built-in views).

Our options are roughly:

  • Option 0: Not have anything built-in (remove the current logger and DebugView), instead redirect people toward 3rd-party views like flexi-logger-view. One major advantage is that we are unaffected by semver-bumping updates of the log-related dependencies (otherwise we'd have to bump the Cursive dependency, even if it's behind a feature flag - and bumping the dependency means every 3rd-party view needs to update as well).
  • Option 1: Have a super-minimal implementation to be used as a last resort for people who just want some debugging right now. This is pretty much the current state.
  • Option 2: Have a good-enough implementation that should satisfy most users, while letting them use 3rd-party views if they prefer.

I'm actually considering all 3 options. One problem with the option 2 is that I don't know exactly what qualifies as "good-enough".

(Note that being able to select the log level should definitely be part of the minimal implementation, so this at least should most likely be merged.)

For now I think I'll aim to stay with option 1, the minimal implementation. This would include selecting the desired log level, but probably not complex templating. In this case we'll want to always enable log level selection, no need for feature flag.

@kalkin
Copy link

kalkin commented Apr 8, 2021

I like Option 2 quite well. I worked previously with pythons prompt_toolkit and this is the feature I most missed. The only feature missing is configuring the loglevel.

@saptarshikar
Copy link

Is there any update on this? Is using cursive-flexi-logger-view the current recommended approach

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.

4 participants