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

Make logging.yml read by default #3801

Closed
noklam opened this issue Apr 11, 2024 · 1 comment · Fixed by #3831
Closed

Make logging.yml read by default #3801

noklam opened this issue Apr 11, 2024 · 1 comment · Fixed by #3831
Assignees

Comments

@noklam
Copy link
Contributor

noklam commented Apr 11, 2024

Quick summary:

  • ✅There are at least one obvious user problem - which is logging get chosen as a tool but has no effect, and the consensus is that we should fix it. - How to enable logging automatically #3446
    • One possible solution is hard coding the path to conf/logging.yml with option to override with KEDRO_LOGGINGI_CONFIG, very similar to be3d28d
  • It's unclear whether we should do more for logging, such as adding a new -v flag
  • It's mentioned logging in development and logging in production are very different, and users probably have different needs:
    • It was mentioned we may want to de-couple rich as a plugin, pip-uninstallable to deactivate it or at least provide an easy enough way to opt out from it.
  • We may consider improving the messaging of logging and people should seek for production grade logging.
  • It was mentioned even though there are users need, Kedro can still say NO as we had for many other things.

Antony shared the old user research here in case you miss it at the end: #2281 (comment)

Originally posted by @noklam in #3755 (comment)

Context

Since 0.19, we introduced a few changes:

This create a bad UX because:

  • New users expect logging.yml will be used by default
  • Existing users migrate from 0.18 -> 0.19 things start working unexpectedly

The scope of ticket is to implement a fix to make sure the default file is read. KEDRO_LOGGING_CONFIG will take priority when it is provided. (See be3d28d for inspiration)

Solution

be3d28d is a good starting point. At the minimal case, the happy path of

  • kedro new --tools=log
  • kedro run should auotmatically read logging.yml

There are things to consider:

  • Should we consider accepting logging* or searching for subfolder?
  • We need to consider corner case where logging.yml is at CONF_SOURCE/logging.yml, by default CONF_SOURCE is conf but user has the option to change it. (Not handled in the old PR)
    • You may find that you cannot access CONF_SOURCE because it is not read yet when logging is initialised, does it takes a lot of changes to make it possible? Is there workaround?

@merelcht merelcht moved this to To Do in Kedro Framework Apr 11, 2024
@merelcht merelcht added this to the Improve logging experience milestone Apr 15, 2024
@SajidAlamQB SajidAlamQB moved this from To Do to In Progress in Kedro Framework Apr 23, 2024
@SajidAlamQB SajidAlamQB moved this from In Progress to In Review in Kedro Framework Apr 30, 2024
@SajidAlamQB
Copy link
Contributor

Completed in #3831

@github-project-automation github-project-automation bot moved this from In Review to Done in Kedro Framework May 8, 2024
@noklam noklam linked a pull request May 21, 2024 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants