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

Optional default logging configuration during import of Kedro Session #1930

Closed
vilozio opened this issue Oct 12, 2022 · 4 comments
Closed

Optional default logging configuration during import of Kedro Session #1930

vilozio opened this issue Oct 12, 2022 · 4 comments
Labels
Issue: Feature Request New feature or improvement to existing feature

Comments

@vilozio
Copy link
Contributor

vilozio commented Oct 12, 2022

Description

When I import KedroSession - from kedro.framework.session import KedroSession - inside the package occurs an import from kedro.framework.project package, where default_logging.yml is configured during package initialization.

This configuration cannot be skipped and if logging was already configured before the import, it will be overwritten.

I suggest to make the default logging configuration as an option.

Context

I use Kedro to write data engineering pipelines and they are executed in Airflow as tasks.

I made my own KedroOperator to run Kedro nodes as Airflow tasks, copied from kedro-airflow plugin with small adjustments.

My Airflow configuration has a custom logging to save task logs to Google Cloud Storage (followed from Airflow documentation guide).

When I import KedroSession, the logging config is overwritten which stops saving of logs to Google Cloud Storage.

Possible Implementation

Introduce an environment variable which flags that the default logging configuration should be skipped, e.g. KEDRO_SKIP_DEFAULT_LOGGING.

Possible Alternatives

Do not configure logging during package initialization, and leave the logging configure explicitly in places where it is needed, for example in CLI.

@vilozio vilozio added the Issue: Feature Request New feature or improvement to existing feature label Oct 12, 2022
@merelcht merelcht added the Community Issue/PR opened by the open-source community label Oct 17, 2022
@merelcht merelcht added this to the Improve the setup of logging milestone Nov 14, 2022
@merelcht merelcht removed the Community Issue/PR opened by the open-source community label Nov 14, 2022
@antonymilne antonymilne self-assigned this Jan 13, 2023
@antonymilne
Copy link
Contributor

Hi @vilozio, thanks very much for raising this issue. I'm going to try and figure out a way to fix and some related issues to do with the kedro default logging mechanism.

Just for now, in order to understand better:

  • the default_logging.yml sets disable_existing_loggers: False so that in theory it shouldn't remove any previously setup loggers. I'm not familiar with logging on airflow, but can you work out why that's not working as intended? Is your problem that the kedro loggers are not being disabled or that kedro is actually removing your airflow logging setup?
  • given that the airflow setup seems to use a logging configuration file, is it not possible to achieve this by modifying your conf/base/logging.yml?

@astrojuanlu
Copy link
Member

With the recent changes in logging, does this issue still stand? @noklam @merelcht

@merelcht
Copy link
Member

I think with the introduction of customisable logging (https://docs.kedro.org/en/stable/logging/index.html#how-to-customise-kedro-logging) this indeed has been solved. It's a slightly different solution, but as far as I understand it should make it possible to not overwrite your existing logging now.

@astrojuanlu
Copy link
Member

Let's close it then, @vilozio if you disagree please drop a comment and we'll hapily reconsider.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Feature Request New feature or improvement to existing feature
Projects
Archived in project
Development

No branches or pull requests

4 participants