-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
[AIRFLOW-1879] Handle ti log entirely within ti #2837
Conversation
Previously logging was setup outside a TaskInstance, this puts everything inside. Also propery closes the logging.
|
||
hostname = socket.getfqdn() | ||
log.info("Running on host %s", hostname) | ||
log.info("Running %s on host %s", ti, hostname) |
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.
Think you want ti.log.info
here.
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.
Oh maybe not. I thought log
wasn't a local variable anymore but tests pass so :)
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.
log is actually a local variable as well. I thought it made more sense to log in that facility rather than 'hijack' the TI log.
Codecov Report
@@ Coverage Diff @@
## master #2837 +/- ##
==========================================
- Coverage 74.01% 73.94% -0.07%
==========================================
Files 159 159
Lines 12076 12075 -1
==========================================
- Hits 8938 8929 -9
- Misses 3138 3146 +8
Continue to review full report at Codecov.
|
@Fokko PTAL |
I like it, we don't want any logging logic inside of the cli class. |
Previously logging was setup outside a TaskInstance, this puts everything inside. Also propery closes the logging. Closes apache#2837 from bolkedebruin/AIRFLOW-1879
Previously logging was setup outside a TaskInstance, this puts everything inside. Also propery closes the logging. Closes apache#2837 from bolkedebruin/AIRFLOW-1879
Make sure you have checked all steps below.
JIRA
Description
Previously logging was setup outside a TaskInstance,
this puts everything inside. Also propery closes
the logging.
Tests
Covered by current tests.
Commits
My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
Passes
git diff upstream/master -u -- "*.py" | flake8 --diff
cc: @ashb @Fokko @jgao54