[AIRFLOW-244] Modify hive operator to inject analysis data #1607
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
paulbramsen
force-pushed
the
paulbramsen/modify_hive_operator_to_inject_analysis_data
branch
from
f759942
to
888ef8b
Compare
Current coverage is 64.07%@@ master #1607 diff @@
==========================================
Files 120 121 +1
Lines 8443 8462 +19
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 5419 5422 +3
- Misses 3024 3040 +16
Partials 0 0
|
paulbramsen
force-pushed
the
paulbramsen/modify_hive_operator_to_inject_analysis_data
branch
2 times, most recently
from
3dc61f3
to
3b32e4b
Compare
| @@ -107,6 +115,10 @@ def run_cli(self, hql, schema=None, verbose=True): | |||
| if conn.password: | |||
| cmd_extra += ['-p', conn.password] | |||
|
|
|||
| if hive_conf: | |||
There was a problem hiding this comment.
Wait why not? If hive_conf is None won't this cause issues?
There was a problem hiding this comment.
You can make hive_conf default to dict()/frozendict() (and pass in dict() instead of None in hive_operator.py)
|
LGTM once landscape check is green. Might get another +1 from Max just in case. |
| import unicodecsv as csv | ||
| import logging | ||
| import re | ||
| import subprocess | ||
| from frozendict import frozendict |
There was a problem hiding this comment.
We shouldn't need that, let's not bring a dep on an external lib just for this.
paulbramsen
force-pushed
the
paulbramsen/modify_hive_operator_to_inject_analysis_data
branch
3 times, most recently
from
bfd79fd
to
ace79c0
Compare
|
code-quality is a false-positive, merging. |
|
A little unit test? |
|
@artwr I'll add some tests in a new PR |
|
Our code coverage thanks you :) |
paulbramsen
deleted the
paulbramsen/modify_hive_operator_to_inject_analysis_data
branch
aoen
pushed a commit
to aoen/incubator-airflow
that referenced
this pull request
Aug 5, 2016
Testing Done: Test dags were run as backfills on an Airbnb Airflow dev box. This PR exposes task/dag id/run data through the HiveOperator for ingestion by performance analysis tools like Dr. Elephant. Closes apache#1607 from paulbramsen/paulbramsen/modify_hive_operator_to_inject_analysis_data
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Dear Airflow Maintainers,
Please accept this PR that addresses the following issues:
Testing Done:
Test dags were run as backfills on an Airbnb Airflow dev box.
This PR exposes task/dag id/run data through the HiveOperator for ingestion by performance analysis tools like Dr. Elephant.
As per discussion this supersedes #1594
@krishnap @artwr @aoen @mistercrunch