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 deprecation warning to ColorHandler #1535

Merged
merged 4 commits into from
May 17, 2022

Conversation

SajidAlamQB
Copy link
Contributor

@SajidAlamQB SajidAlamQB commented May 16, 2022

Signed-off-by: SajidAlamQB [email protected]

Description

RichHandler will become the default logger, we need to remove the ColorHandler provided in kedro.extras.logging for Kedro 0.19.0. This PR will add deprecation warnings to warn users of this fact.

Related Issue: #1515

Development notes

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the RELEASE.md file
  • Added tests to cover my changes

Signed-off-by: SajidAlamQB <[email protected]>
@SajidAlamQB SajidAlamQB self-assigned this May 16, 2022
@SajidAlamQB SajidAlamQB requested a review from idanov as a code owner May 16, 2022 09:46
@SajidAlamQB SajidAlamQB linked an issue May 16, 2022 that may be closed by this pull request
Copy link
Contributor

@antonymilne antonymilne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please could you try specifying ColorHandler in your logging.yml file and see what this looks like when you do a kedro run? I wonder whether it will completely fill the screen with warnings and we will need to use a different filter.

kedro/extras/logging/__init__.py Outdated Show resolved Hide resolved
RELEASE.md Outdated Show resolved Hide resolved
Signed-off-by: SajidAlamQB <[email protected]>
@SajidAlamQB
Copy link
Contributor Author

SajidAlamQB commented May 16, 2022

Please could you try specifying ColorHandler in your logging.yml file and see what this looks like when you do a kedro run? I wonder whether it will completely fill the screen with warnings and we will need to use a different filter.

This is what the warning looks like when we do kedro run.

color

Copy link
Contributor

@antonymilne antonymilne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for testing it out 👍

Copy link
Contributor

@AhdraMeraliQB AhdraMeraliQB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job!

Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 We should make sure the issue isn't completely closed when this is merged, and tag it with 0.19.0.

@SajidAlamQB SajidAlamQB modified the milestone: 0.19.0 May 17, 2022
@SajidAlamQB SajidAlamQB merged commit 8cc588e into main May 17, 2022
@SajidAlamQB SajidAlamQB deleted the Add-deprecation-warning-to-`ColorHandler` branch May 17, 2022 11:35
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