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

[RFC]: Pipeline definition merging #816

Open
dannyvv opened this issue Nov 7, 2024 · 0 comments
Open

[RFC]: Pipeline definition merging #816

dannyvv opened this issue Nov 7, 2024 · 0 comments
Assignees

Comments

@dannyvv
Copy link
Member

dannyvv commented Nov 7, 2024

This RFC is part of a larger work proposal: #814

Overview

Today when lage processes pipeline configurations are defined, it is a last value wins model.
For example the lage.config.js file in the Lage repo has a definition for all tests:

test: {
      inputs: ["**/_test/**"], 
      dependsOn: ["build"],
    },

Later in the file it 'specializes' it for one project:

    "@myOrg/MyPackage#preTestCodeGen": {
    }
    "@myOrg/MyPackage#test": {
      weight: 5,
      dependsOn: ["preTestCodeGen"],
    },

In this concrete example it extends the #test target for one project with extra details.
The current implementation it is an overwrite operation. As in the thing that was declared for all
test targets (the inputs and dependsOn "build" target are ignored and overwritten for that project.
If there are a few customizations that might be okay, but in large monorepo's we expect quite a few. Especially if we
are going to include predicted inputs and outputs.
The reverse will also be expensive. What if we update the shared build scripts to produce another file (say code coverage file) we need to declare that as an output. When we do we would have to hunt down all the customizations as well and add it there.

So it is obvious that for large monorepo's it is desirable to perform a deepMerge of the pipeline configurations.
This would be a breaking change so I understand that this will have to be an opt-in feature.

Detailed Design

Add a configuration flag for this to lage.config.js

{
   mergePipelineDefinitions: true
}

Thread this value through to the WorkspaceTargetGraphBuilder class. and update the addTargetConfig function.
This function would check if a target already existsed and would call a deepMerge function to recursivly merge them before adding them to the graph with the same function that overwrites.

There are various libraries to deal with deepMerge. I have no strong preference either way which one or a simple roll-your-own. I'll let @kenotron make the call here.

Test Plan

regular UT's

Performance, Resilience, Monitoring

deepMerge has issues with cyclic object trees, some of the libraries support failing in that case which is what I would like to do.

Security & Privacy

N/A

Accessibility

N/A

World Readiness

N/A, configs are not localized.

@dannyvv dannyvv changed the title [RFC]: lage.config.js merging [RFC]: Pipeline definition merging Dec 14, 2024
dannyvv added a commit to dannyvv/lage that referenced this issue Dec 18, 2024
dannyvv added a commit to dannyvv/lage that referenced this issue Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants