Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 the Kedro RichHandler #2592
Add the Kedro RichHandler #2592
Changes from all commits
db214c2
d817420
6a31eff
2b02951
df7308e
c507984
3ba89c9
0fb70c3
2dbc0c0
05eaa91
7b9a989
8e531f0
810d25b
fc0d519
1e5d14a
85ff28d
1b09078
1fffdc0
f9dffc2
f5be2d4
044ccda
23f6427
04571ee
ea23b38
8894aa5
5efefd8
9d65959
8b4f63a
0c166ad
b81a5ea
8f2788a
f263931
5071f5a
f9d5769
df58139
af4a9ce
d123a3e
6ad4463
632d29e
b883906
e749dca
c726d6e
af3e3d0
40e9156
b7d94e6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like
settings.py
, we could have some commented default here to help user discover possible options. I putshow_locals
because initially when we introducerich
, we want to enable it asTrue
. It turns out to be a bit noisy for some users, and there was no good way to turn it off by the user.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this idea!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also like the idea, but remember this file won't exist in the default template in 0.19 and it will move to docs instead - see #2426 (this was a relatively recent decision in tech design: #2281 (comment)).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also note you changed the
test_starter
here, not the actual kedro template starter.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This creates name collision with
kedro.logging
, I don't think this is needed at all. It was there in the first commit in 0.14There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have considered doing this many times but wasn't brave enough to do so in case it broke something, because I changed something small with logging before and it broke things unexpectedly...
Happy to have it removed but please do a good manual test! I did so yesterday before you removed these lines and everything worked ok, but would be great to double check now 🤞
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rich's traceback won't be installed when
kedro.framework.project
is imported, it's now just a logging handler. This is an improvement because importing a module no longer has side-effect.