move debug logs to debug channel#2163
Conversation
pankajastro
left a comment
There was a problem hiding this comment.
Thanks for the PR! It looks like the CI checks are failing—could you please take a look and fix them?
ed66a5b to
536a262
Compare
536a262 to
b244f00
Compare
|
hello @pankajastro tests should be good now, just need your approval to execute tests again on CI |
|
@pankajastro is this normal that my CI is blocked? Thanks |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2163 +/- ##
=======================================
Coverage 97.81% 97.81%
=======================================
Files 93 93
Lines 6006 6006
=======================================
Hits 5875 5875
Misses 131 131 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Yes, for security reasons and to avoid potential misuse, CI requires approval. |
|
I know the duplicated log lines aren’t ideal, but I think it’s important to understand why they’re happening. |
I tried running a few DAGs and couldn’t see duplicate logs. We may need a broader discussion about changing the log level and deciding what we should and shouldn’t log. |
|
These logs are helpful for many users—they make it easier to understand what’s happening under the hood and also give a rough idea of performance. PR #2235 adds |
| logger.debug("Total nodes: %i", len(self.nodes)) | ||
| logger.debug("Total filtered nodes: %i", len(self.filtered_nodes)) |
There was a problem hiding this comment.
Maybe we can combine both and keep keep as info
|
Closing since PR: #2235 has improved the readability, and we will cut alpha soon for testing |
Description
These logs are displayed on every task multiple times. They hide meaningful information logged in the tasks.
There are more useful when trying to debug performance issues.
Here is an example of what is shown in my tasks, this makes the logs less readable
Related Issue(s)
closes #2069
Breaking Change?
This could be a breaking change is anyone is using these logs programatically but I doubt that is the case
Checklist