-
Notifications
You must be signed in to change notification settings - Fork 88
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
feat: Column Lineage API #280
Conversation
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>
Codecov Report
@@ Coverage Diff @@
## master #280 +/- ##
==========================================
+ Coverage 74.10% 77.61% +3.51%
==========================================
Files 25 27 +2
Lines 1255 1385 +130
Branches 136 163 +27
==========================================
+ Hits 930 1075 +145
+ Misses 297 263 -34
- Partials 28 47 +19
Continue to review full report at Codecov.
|
cc: @youngyjd @dkunitskiy |
hey @allisonsuarez, I wonder how you test it, is it by building a sub private class with CW to use the API directly? |
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.
Code LGTM
direction = args.get('direction', 'both') | ||
depth = args.get('depth', 0) | ||
try: | ||
lineage = self.client.get_lineage(id=f"{table_uri}/{column_name}", |
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.
If this id generation is a common pattern, should it be handled by a util or wrapper?
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.
good question, I don't think we have any generic method for both tables and columns, in any methods that call columns like get_column_description on neo4j proxy we get column through its association with table name node, but if we have the entire column key we can just call directly with the key rather than querying neo4j for table->column->what_you_want
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.
ah, yeah -- i guess ultimately this is tied to ColumnMetadata.COLUMN_KEY_FORMAT
-- we're "getting lucky" in that column shares a prefix with TableMetadata.TABLE_KEY_FORMAT
. I'm sure there are other places that rely on this behavior
In this case I just used a private subclass for neo4j proxy and returned some mock data from the get_lineage method. Still working on getting column lineage from CW |
Summary of Changes
Tests
What tests did you add or modify and why? If no tests were added or modified, explain why. Remove this line
Documentation
What documentation did you add or modify and why? Add any relevant links then remove this line
CheckList
Make sure you have checked all steps below to ensure a timely review.
make test