Change Sets to enable dry runs and to standarize the file management API

[nicholasyager]

Overview

In our current implementation of dbt-meshify, we've tightly coupled our logical operations (adding a group, adding a contract, etc.) together with our file loading tooling (DbtFileManager) and our file editing tooling (DbtMeshFileEditor). This approach works, but there are a few distinct drawbacks:

1. Each file editing operation happens within the context of the logical operation being run. This means if an operation fails, we may be in an undefined state. Also, we may be reading and writing the same yaml file if there are multiple models being modified if they are patched in the same file.
2. As a developer, it can be unclear how to add new functionality. The workflow is not well-defined, and the bespoke nature of each component means that there can be non-trivial differences between file editing functionality.
3. It's not possible to communicate to a use what dbt-meshify will be doing prior to executing the changes (no dry runs).

To mitigate these issues, I propose we create a concept of a Change, which tracks operations that need to be performed on a given resource. Each logical operation will identify a set of Changes that need to be made (e.g. creating a Group and updating a Model to be a member of the new Group). These sets of Changes will then be merged together and processed as a single unit following a consistent API.

Example

For example, suppose we're creating a new revenue group, and updating the orders model to be a member of that group. For this, the ResourceGrouper would yield the following changes:

[
    {
        'type': ChangeType.add,
        'resource_type': ResourceType.group,
        'identifier': 'group.project_name.revenue',
        'config': {
            'name': 'revenue',
            'owner': {
                'name': 'Dave,
                'email': '[email protected]'
            }
        } 
    },
    {
        'type': ChangeType.update,
        'resource_type': ResourceType.model,
        'identifier': 'model.project_name.orders',
        'config': {
            'group': 'revenue',
        } 
    }
]

Within the cli method, we could then receive the change set from each downstream CLI operation and pass the change set to a change set processor (ChangeSetProcessor 😉) which merges like operations (two + updates to the same file/model/etc), and passes the operation to an appropriate file handling class (ResourcePropertiesEditor, ProjectEditor, SQLEditor, PythonEditor), each of which following a standard Protocol (create(), update(), move()).

We can also update the cli method to accept a --dry-run option, which prints a pretty set of changes to be applied and returns prior to accepting modifications. If we wanted to be really mad lads, we could also accept change sets as input, thereby allowing for a code review of what dbt-meshify will be executing.

This all should sound familiar to folks familiar with terraform >.>

Benefits

• Clearer lines of responsibility for different parts of the tool. What part identifies work to be done, and which part modifies the file system.
• Enables dry-runs!
• Decoupling the logical operations and file operations greatly simplifies testing. We can independently prove that the logical operation is selecting the correct operations without needing to check the yaml generated, and we can test the yaml generated without needing a specific dbt project to exist.

Drawbacks

• This approach will add a fair amount of boilerplate for each file type that we want to execute.
• This type of decoupling may require a fair amount of rewriting for our file management code.

Open questions

• How does this approach work with project creation?
• Are there rough edges w/r/t nested projects?
• How much optimization should the ChangeSetProcessor perform in our initial implementation?

[dave-connors-3]

I really like this idea a whole lot -- being able to create a plan before you execute something is good architectural and general life advice.

i don't think I have a ton to say that's insightful and not implementation/details focused, but i'll think out loud anyway!

• Which of our current classes are most likely to be deprecated by these changes? Certainly the DbtMeshFileEditor and DbtMeshFileEditor and DbtMeshConstructor as you note would be simplified and split to suit the various filetypes. Do you see any other ovbious ones that would need wholesale rewrites?
• Related to the above, do you see places in the codebase where the Class abstractions don't quite pass the smell test? Would this rewrite give us the opportunity to restructure some of those relationships as well?
• Very excited to think about generating a change in yml, and then just passing that as the argument to the editing function -- will make the editing functions so much cleaner and less repetitive! probably only need one function in fact! what a world!

How does this approach work with project creation?

Would it make sense for the Change class to represent resource level and project level edits? could Change.resource_type be a project or a resource? or does that confuse the level of abstraction?

Are there rough edges w/r/t nested projects?

I would assume just the same rough edges we have now, namely keeping track of when and how to write to a subdirectory vs the source project in the course of a split operation. Not sure where that info would live -- likely not still in the DbtFileManager class?

[nicholasyager]

Do you see any other ovbious ones that would need wholesale rewrites?

I don't think there's anything else that requires a full rewrite. Probably a number of test changes, and maybe some tweaks to how our logical operations function to keep things clean, however. It'll probably be a chonky change.

do you see places in the codebase where the Class abstractions don't quite pass the smell test?

I think this will approach will start to highlight some of the oddities around how our logical operations are currently being defined (grouping vs contracts vs versions, etc). Thankfully, massaging those can be in follow-up issues, IMHO.

Would it make sense for the Change class to represent resource level and project level edits? could Change.resource_type be a project or a resource? or does that confuse the level of abstraction?

I suspect this is oddness is the result of naming. Perhaps entity_type should include both Resources (Group, Model, etc), as well as Projects.
... this convo sounds familiar 😁

I would assume just the same rough edges we have now, namely keeping track of when and how to write to a subdirectory vs the source project in the course of a split operation. Not sure where that info would live -- likely not still in the DbtFileManager class?

Yeah, project-level context might need to be tracked somewhere specifically.