Fix ambiguous reference error for duplicate model names across packages with tests #8488
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
|
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
Codecov Report
@@ Coverage Diff @@
## main #8488 +/- ##
==========================================
- Coverage 86.36% 86.30% -0.06%
==========================================
Files 174 174
Lines 25568 25573 +5
==========================================
- Hits 22081 22071 -10
- Misses 3487 3502 +15
Flags with carried forward coverage won't be shown. Click here to find out more.
|
MichelleArk
changed the title
MichelleArk
requested review from
martynydbt and
gshank
and removed request for
martynydbt
gshank
approved these changes
Aug 24, 2023
4 tasks
peterallenwebb
pushed a commit
that referenced
this pull request
Aug 30, 2023
peterallenwebb
added a commit
that referenced
this pull request
Aug 30, 2023
* Add compiled node properties to run_results.json * Include compiled-node attributes in run_results.json * Fix typo * Bump schema version of run_results * Fix test assertions * Update expected run_results to reflect new attributes * Code review changes * Fix mypy warnings for ManifestLoader.load() (#8443) * revert python version for docker images (#8445) * revert python version for docker images * add comment to not update python version, update changelog * Bumping version to 1.7.0b1 and generate changelog * [CT-3013] Fix parsing of `window_groupings` (#8454) * Update semantic model parsing tests to check measure non_additive_dimension spec * Make `window_groupings` default to empty list if not specified on `non_additive_dimension` * Add changie doc for `window_groupings` parsing fix * update `Number` class to handle integer values (#8306) * add show test for json data * oh changie my changie * revert unecessary cahnge to fixture * keep decimal class for precision methods, but return __int__ value * jerco updates * update integer type * update other tests * Update .changes/unreleased/Fixes-20230803-093502.yaml --------- Co-authored-by: Emily Rockman <emily.rockman@dbtlabs.com> * Improve docker image README (#8212) * Improve docker image README - Fix unnecessary/missing newline escapes - Remove double whitespace between parameters - 2-space indent for extra lines in image build commands * Add changelog entry for #8212 * ADAP-814: Refactor prep for MV updates (#8459) * apply reformatting changes only for #8449 * add logging back to get_create_materialized_view_as_sql * changie * swap trigger (#8463) * update the implementation template (#8466) * update the implementation template * add colon * Split tests into classes (#8474) * add flaky decorator * split up tests into classes * revert update agate for int (#8478) * updated typing and methods to meet mypy standards (#8485) * Convert error to conditional warning for unversioned contracted model, fix msg format (#8451) * first pass, tests need updates * update proto defn * fixing tests * more test fixes * finish fixing test file * reformat the message * formatting messages * changelog * add event to unit test * feedback on message structure * WIP * fix up event to take in all fields * fix test * Fix ambiguous reference error for duplicate model names across packages with tests (#8488) * Safely remove external nodes from manifest (#8495) * [CT-2840] Improved semantic layer protocol satisfaction tests (#8456) * Test `SemanticModel` satisfies protocol when none of it's `Optionals` are specified * Add tests ensuring SourceFileMetadata and FileSlice satisfiy DSI protocols * Add test asserting Defaults obj satisfies protocol * Add test asserting SemanticModel with optionals specified satisfies protocol * Split dimension protocol satisfaction tests into with and without optionals * Simplify DSI Protocol import strategy in protocol satisfaction tests * Add test asserting DimensionValidtyParams satisfies protocol * Add test asserting DimensionTypeParams satisfies protocol * Split entity protocol satisfaction tests into with and without optionals * Split measure protocol satisfication tests and add measure aggregation params satisficaition test * Split metric protocol satisfaction test into optional specified an unspecified Additionally, create where_filter pytest fixture * Improve protocol satisfaction tests for MetricTypeParams and sub protocols Specifically we added/improved protocol satisfaction tests for - MetricTypeParams - MetricInput - MetricInputMeasure - MetricTimeWindow * Convert to using mashumaro jsonschema with acceptable performance (#8437) * Regenerate run_results schema after merging in changes from main. --------- Co-authored-by: Gerda Shank <gerda@dbtlabs.com> Co-authored-by: Matthew McKnight <91097623+McKnight-42@users.noreply.github.com> Co-authored-by: Github Build Bot <buildbot@fishtownanalytics.com> Co-authored-by: Quigley Malcolm <QMalcolm@users.noreply.github.com> Co-authored-by: dave-connors-3 <73915542+dave-connors-3@users.noreply.github.com> Co-authored-by: Emily Rockman <emily.rockman@dbtlabs.com> Co-authored-by: Jaime Martínez Rincón <jaime@jamezrin.name> Co-authored-by: Mike Alfare <13974384+mikealfare@users.noreply.github.com> Co-authored-by: Michelle Ark <MichelleArk@users.noreply.github.com>
5 tasks
5 tasks
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.
resolves #8327
resolves #8493
docs dbt-labs/docs.getdbt.com/# N/A
Problem
When generic test nodes are constructed, an 'attached_node' attribute is set by looking up the attached node in the manifest via
manifest.ref_lookup.get_unique_id. Currently, this call passesNoneas the value forpackage_name-- resulting in ambiguous reference errors if the model exists with a duplicate name across packages.This is unexpected behaviour since the restriction to have unique model names across packages was lifted in #7374.
Similarly, looking up the unique id from ref_lookup for versioned node parsing also passes in
Nonefor package_name, resulting in an ambiguous ref name error described in #8493.Solution
Pass the target package_name to ref lookup for attached_node, as generic test nodes should only ever be attached to nodes defined in the same package
Checklist