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

Add ability to skip building module #83

Open
dustyhorizon opened this issue May 22, 2018 · 7 comments
Open

Add ability to skip building module #83

dustyhorizon opened this issue May 22, 2018 · 7 comments

Comments

@dustyhorizon
Copy link

dustyhorizon commented May 22, 2018

With a differential build flow set up, sometimes there is a need to not build a particular module. It would be good to have flag / file based token to indicate that this module although defined and with changes, there is no need to build it.

A possible implementation would be to add a --skip flag but that would require changing the CI script everytime. An alternative is to check for a .mbt_skip_module file at the module root and if the file exists, skip building or to define a global .mbt_skip_modules at the repo root with a list of modules not to build.

If any modules that depended on a skipped module, it should either skip or fail gracefully, optimally it should be configurable I guess.

What do you think?

@buddhike
Copy link
Member

Sounds reasonable.

Not sure if you have seen this, but there's already a feature to do the reverse.
https://github.com/mbtproject/mbt/blob/master/doc/mbt_build_commit.md (See --name and --fuzzy options)

What do you think about --exclude flag that is specified along with current --name and --fuzzy options?

Is the motivation behind .mbt_skip_modules to keep track of modules that are ignored?
In our case, we store our CI script in the repo as well. In a setup like that, changing the arguments to mbt would be fully tracked with rest of the source tree. What do you think about that?

In terms of the behaviour of this command, are we thinking of just excluding the specified modules? What about their dependents (that haven't been modified but marked for build because of the changes to dependencies?).

@dustyhorizon
Copy link
Author

dustyhorizon commented May 23, 2018

I'm thinking that an --exclude command would work as well. Not very sure what would be the ideal behavior with regards to skipped modules although what I had in mind was that it could either 1) build the version (commit) of that module that does not have the .mbt_skip_module / listed in .mbt_skip_modules, 2) in the event --exclude feature were to be added, mark the module was built / show a warning that it was skipped or 3) to provide a flag that forces a build failure upon requiring a skipped module.

I think further consideration is required so that this feature could benefit most pipelines without requiring some sort of funny hack to skip building something because it is not ready.

How it is implemented on my end was as part of the build process, the CI would check for the presence of a file and if the file exists, automatically mark as complete and move on. Modules that depend on the skipped module will naturally fail since it cannot access a function / artifact from the skipped module. Though we do have modules that does not explicitly require the skipped module and could still continue building (e.g. docker images since unavailable locally due to a skipped build will be pulled from our private registry instead).

Hope this provides some clarity.

@buddhike
Copy link
Member

Thanks for the detailed explanation. I've been giving some thought into this and definitely think this feature should go into the product.

May be the details of how to handle the dependencies etc would fall out if we discuss the scenarios that would benefit from this.
I feel that, we are seeking a --skip feature due to the lack of filtering available in build (even --name option is too verbose to deal with in a decent size repo).

One scenario I could think of is, when we have different pipelines for sub-trees. For example, consider a setup like this:

.
|-- Website1
|     |-- module-a
|     |-- module-b
|-- Website2
|     |-- module-c
|     |-- module-d
|-- Shared
|    |-- shared-mod

If we don't want to trigger the pipeline for Website1 when there's a change in Website2, we may want to configure it using something like mbt build xxx --subtree Website1. That would build anything that should be built due to a change inside Website1 subtree or result of a dependency change (i.e. shared-mod is modified and module-a depends on it).

IMHO, model like this would be a bit more predictable than --skip. With --skip, CI script (or ignore files) will dictate what's getting ignored and may not be immediately apparent to the team. For example, consider the change to shared-mod again. If CI script had an exclusion rule for shared-mod, the artifact produced for module-a may never work until it is built.

Hopefully am I not going off track here? Are you able to share the details of your scenario?

@buddhike
Copy link
Member

@dustyhorizon Sorry, re-reading this thread clarified your use case.

I think further consideration is required so that this feature could benefit most pipelines without requiring some sort of funny hack to skip building something because it is not ready.

In this case, I like the idea of storing this instruction in a file. Could we use a new property in spec (.mbt.yml) instead of requiring a new file?

Something like:

exclude: true

And at build time by default, we exclude all modules with exclude:true or anything that depends on them. We could also improve the build summary by displaying the reason for skipping a build.

I think skipped modules (or their dependents) should not be considered during apply command. Otherwise, you could generate deployment documents relying on artifacts that are not built yet.

Finally, I think a switch --ignore-exclude should be provided for developers using mbt locally.

Keen to hear your thoughts around not building the whole chain.

@dustyhorizon
Copy link
Author

Sorry for the late response, my use case was that for example when I am developing new modules / images, sometimes I do not want them to be built as they are incomplete. I would preferably have mbt manage the DONOTBUILD part and hence this feature request. Currently I am doing a roundabout method of checking if a file exists and if so, instantly complete the build for that module (a Makefile was involved so checks coule be done / complete builds if skipped). I suppose its ugly but it works haha.

The exclude flag for mbt would be ideal, I am thinking if someone have depended on a excluded module it should fail fast as it could produce incomplete artifacts? Maybe an additional flag optional could be added so that builds can continue if a build depended on an optional module?

@buddhike
Copy link
Member

@dustyhorizon Thanks for further input. This is a reasonable request and I've already marked it as an enhancement (which is an indication that this would be added in a future release).

Failing the build might not be desirable in some situations. For example, if you don't have a good QA process for your PRs, a simple misconfiguration in mbt file could lead to broken master. That's why I thought excluding the entire dependency chain would more forgiving. In build summary, we can output the list of skipped modules and the reason for skipping to make it more obvious to the user.

One of the main goals of mbt is to make repositories describe modules using the metadata stored within. We could then use that metadata to generate various documents such as deployment manifests as of a particular commit sha. If we introduce an optional flag, I wonder whether it's going to diminish the correctness aspect of the tool (for example, imagine, you build with optional and then generate a deployment manifest that has references to images that are not built yet. You may have to wait for the deployment failure be fully aware of the problem).

Let me know if this makes sense.

PS: Did you consider simply not creating .mbt.yml in module directories you want to skip? Might be a less hacky option until we get this feature out ;-).

@dustyhorizon
Copy link
Author

Haha actually the hacky skip feature was in place before I found out about mbt and I really wanted break away from these hacky fixes hence the feature request. Thanks for the hard work though!

Understand where you are heading with regards to the optional flag. No worries man, the exclude feature would already be very helpful.

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