fix(context): normalize SPN/objectClass to a list before dlt persistence - #12
Open
chryzsh wants to merge 1 commit into
Open
fix(context): normalize SPN/objectClass to a list before dlt persistence#12chryzsh wants to merge 1 commit into
chryzsh wants to merge 1 commit into
Conversation
ldap_resolved_principals failed to persist entirely against a real environment: dlt.common.schema.exceptions.DataValidationError, "Can't add variant column service_principal_name__v_text ... data_types are frozen". Reproduced identically across two separate fresh output directories, so not stale state -- a deterministic bug in this environment's real data. ADClient._entry_to_dict collapses a multi-valued LDAP attribute to a bare scalar when there's exactly one value, and to None when absent, keeping a list only for two-or-more. servicePrincipalName's cardinality genuinely varies per-principal in a real domain (a user often has 0 or 1, a site server carries many), so _record_resolved_principal was hand`ing dlt None, a bare string, and a list for the same column across different rows in one run -- guaranteeing the frozen-contract's variant-column rejection, which fails the whole load job, not just the offending row. Wiped AD node naming (44/50 nodes emitted as bare stubs) and the SPN-based MSSQL-server fallback. Fix: new _as_multivalued_list() helper, applied to both service_principal_name and object_class (identical collapsing bug, rarely triggered since objectClass is almost never single-valued) before recording -- matches what transforms.py's ad_props builder already expected for both columns. contract_mode=freeze itself is correct and untouched; the bug was upstream data-shape inconsistency. Ticket: con-8bed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
ldap_resolved_principalsfailed to persist entirely against a real environment:Reproduced identically across two separate, fresh output directories, ruling out stale pipeline state.
dlt infers a JSONL column's type from the rows it sees and, when a later row needs a different type, normally adds a
<col>__v_<type>variant column to hold it.ldap_resolved_principals's schema contract iscontract_mode=freeze(deliberately, to catch data-shape regressions early), so the promotion is rejected and the entire load job fails, not just the offending row. Zero rows land for the whole table.Root cause:
ADClient._entry_to_dict(clients/ad.py:201,values if len(values) > 1 else values[0]) collapses any LDAP multi-valued attribute to a bare scalar when there's exactly one value (an absent attribute is set toNoneearlier).servicePrincipalNameis inherently multi-valued and its cardinality varies per-principal in any real domain: a user often has 0 or 1, a site server carries many. So across one real collect,context.py's_record_resolved_principalwas handing dltNone, a bare string, and a list for the same column across different rows.Impact: wiped AD node naming (44/50 AD nodes emitted as bare stubs in the affected run) and the SPN-based MSSQL-server discovery fallback.
Fix
New
_as_multivalued_list()helper incontext.py, applied to bothservice_principal_nameandobject_classin_record_resolved_principal, coercingNone/scalar/list to always-a-list before the row is recorded. Matches whattransforms.py'sad_propsbuilder already assumed for both columns.contract_mode=freezeis left untouched. It caught a real bug correctly; the fix is to the upstream data shape, not the contract.Type of Change
Testing
New tests in
tests/ldap_resolved_principals_test.py:test_service_principal_name_scalar_is_normalized_to_list,test_service_principal_name_none_is_normalized_to_empty_list,test_object_class_scalar_is_normalized_to_list,test_resolved_principals_have_consistent_spn_type_across_mixed_cardinality(records principals with 0/1/many SPNs in one run and asserts every recorded row's type is a list). Full suite passes.