-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
fix(ingestion): fix AssertionError in base_transformer #7702
fix(ingestion): fix AssertionError in base_transformer #7702
Conversation
metadata-ingestion/src/datahub/ingestion/transformer/base_transformer.py
Outdated
Show resolved
Hide resolved
I was thinking about this PR a bit more. We should really just extract the entity type from the MCE's urn (using Not sure if you want to take that @sgomezvillamor or if we should leave that for a follow-up |
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #7702 +/- ##
==========================================
- Coverage 74.98% 67.06% -7.92%
==========================================
Files 353 353
Lines 35422 35418 -4
==========================================
- Hits 26560 23753 -2807
- Misses 8862 11665 +2803
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 77 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
@sgomezvillamor thanks! |
Co-authored-by: Sergio Gomez Villamor <[email protected]>
This is fixing an unnecessary validation that is raising an AssertionError.
Context:
We have a transform with the following
entity_types
:In this scenario, when the ingestion pipeline is processing MCE events,
_should_process
raises the exception because of the assert here:container
is not in theself.entity_type_mappings
Instead, in the case of MCE events, not all
entity_types
should be considered for the lookup but the ones included in the mapping.I also take this as an opportunity to extend the entities in the mapping.
Checklist