Skip to content
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

[Artifacts] Use Manifest instead of WritableManifest in PreviousState and _get_deferred_manifest #9567

Closed
1 task done
Tracked by #9099
MichelleArk opened this issue Feb 13, 2024 · 0 comments · Fixed by #9544
Closed
1 task done
Tracked by #9099
Assignees
Labels
artifacts pre-regression Regression not yet in a stable release

Comments

@MichelleArk
Copy link
Contributor

Housekeeping

  • I am a maintainer of dbt-core

Short description

Currently, PreviousState reads in a manifest artifact and uses the resulting WritableManifest object for state comparison downstream. With the artifacts restructuring happening as part of #9099, the WritableManifest class will contain 'resource' classes that don't have the requisite same_contents methods implemented.

Additionally, the _get_deferred_manifest method also returns a WritableManifest when it should now return a Manifest. This will lead to issues during compilation because it is actually possible that a WritableManifest is passed as manifest in link_node, which will break if the depends_on_nodes method is unavailable on resource classes.

Acceptance criteria

  • Update usage of WritableManifest internally (PreviousState, _get_deferred_manifest) to use Manifest by converting from a WritableManifest to internal Manifest.
    • e.g. introduce a Manifest.from_writable_manifest constructor.

Suggested Tests

If there is no test already that includes SemanticModels in a deferred manifest, this would catch this issue because the SemanticModel Resource class does not have a depends_on_nodes.

Impact to Other Teams

N/A

Will backports be required?

No

Context

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
artifacts pre-regression Regression not yet in a stable release
Projects
None yet
1 participant