dbt-core icon indicating copy to clipboard operation
dbt-core copied to clipboard

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

Open MichelleArk opened this issue 1 year ago • 0 comments

Housekeeping

  • [X] 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 https://github.com/dbt-labs/dbt-core/issues/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

MichelleArk avatar Feb 13 '24 18:02 MichelleArk