-
Notifications
You must be signed in to change notification settings - Fork 3k
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
refactor(metadata-ingestion/pattern_cleanup_ownership): add logging f… #12536
base: master
Are you sure you want to change the base?
refactor(metadata-ingestion/pattern_cleanup_ownership): add logging f… #12536
Conversation
…or un-parseable owner urns
Codecov ReportAttention: Patch coverage is
❌ Your patch check has failed because the patch coverage (18.75%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.
... and 45 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
metadata-ingestion/src/datahub/ingestion/transformer/pattern_cleanup_ownership.py
Outdated
Show resolved
Hide resolved
user_id = re.sub(value, "", user_id) | ||
cleaned_owner_urns.append(_USER_URN_PREFIX + user_id) | ||
except IndexError: | ||
log.warning(f"Could not parse {owner_urn} from {entity_urn}") |
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.
can we add some tests that repro the issue?
does it happen when group urns get passed in, or some other situation?
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.
mmm yes that's right, it's when it's a group urn. I never actually noticed that pattern 😅
…group owners and tests
I've added a case for group ownership. I kept the exception logging in case anyone else finds a situation where it won't parse the owner properly. |
…or un-parseable owner urns
I was getting the following error due to some owner URNs not being a valid URN. It would fail our ingestion pipeline. I figure being able to warn about it and move along would be a good solution.
![image](https://private-user-images.githubusercontent.com/56204545/409170709-ff97b821-7394-42ae-af4a-f276cc0e9e15.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk0MjQyMjYsIm5iZiI6MTczOTQyMzkyNiwicGF0aCI6Ii81NjIwNDU0NS80MDkxNzA3MDktZmY5N2I4MjEtNzM5NC00MmFlLWFmNGEtZjI3NmNjMGU5ZTE1LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTMlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjEzVDA1MTg0NlomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTgwODUxOWMxZGU3MTVkZDYzY2IwZWRmM2M5MzQ4MTUzZDFkMjdkMzcyNGIyNTE0OTFlYzFkZjc5OTY2MTNjMTkmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.OB1BPejHIssmOSNXuVfCw2jR1Ta2JsQptVXz9T5I9X4)
Checklist