fix!: Raise exceptions when a plugin fails to load.

[xitij2000]

When a plugin is missing or throws an error while loading, Tutor continues as normal. At times this has made us recognise issues far later than we'd like.

This change will raise an error causing the command to return an error and the break the CI flow rather than deploying a broken instance with missing plugins.

We've considered few alternatives.

1. Put this behind an option flag such as --check, however this flag would need to be added in a lot of places that cause plugins to load.
2. Adding another command like tutor doctor this is a convention that many such tools follow that check the config and environment for errors and suggest fixes. This issue with this is that it will require updating existing usage of tutor to call tutor doctor first.
3. Creating a plugin for this. We think this is a core feature hat should be part of tutor, but this could potentially be done in a plugin.

[xitij2000]

Note this PR is to get the conversation started, we can improve the error messaging etc.

[Kelketek]

@xitij2000 It looks like you can't disable the plugin using the command line after running into this problem. I could try modifying the config, but I can't ask it for the config root, either. Is it possible to keep this a warning for configuration commands but an error for things like image building and launching?

[xitij2000]

@xitij2000 It looks like you can't disable the plugin using the command line after running into this problem. I could try modifying the config, but I can't ask it for the config root, either. Is it possible to keep this a warning for configuration commands but an error for things like image building and launching?

I'll look into that. For now I just wanted to get some feedback. Is there is a reason to not do this at all? Or move it to a separate command or plugin.

[ahmed-arb]

The fail-fast goal makes sense, but raising at the plugin-loading layer is too low: ensure_plugins_enabled runs on every invocation, so a plugin that fails to load doesn't just break launch; it also breaks:

• tutor plugins disable <bad-plugin>
• tutor config
• tutor --help

The commands you'd use to recover all trigger plugin loading first, so the user gets bricked into hand-editing config.yml or running pip uninstall (which is what @Kelketek hit above).

Also worth noting: the raise lives inside the for name in names loop in load_all, so the first failing plugin aborts loading of all remaining plugins and skips PLUGINS_LOADED.do(). As a result, one broken plugin takes down healthy ones too.

I'd suggest keeping loading (warn + record failures) and moving the hard failure up to deployment/build commands (launch, start, images build) while leaving inspection/recovery commands as warnings.

[xitij2000]

@ahmed-arb Thanks for having a look!

I agree with all the points you have raised, I just wanted to start discussing this issue with this PR. I plan to make the solution more robust when I get back to this.

[Kelketek]

Oh, cool. I didn't know you could catch sys.exit like this.

[Kelketek]

@xitij2000 Thanks for these updates. Image building succeeds, though it should be one of the commands that fails if an enabled plugin is missing, in my opinion. Actually, launch also gets off the ground right now. Can you double-check that things are actually failing for you where they need to be?

CI is also failing.

You noted to me over our internal ticket that this set of errors would cause a freshly installed tutor launch to fail due to installing only tutor leaving out plugins like mfe and indigo. However, I actually think this is preferable-- if someone accidentally installs only tutor and launches a brand new install, they're not following the tutorial closely and have almost certainly made a mistake.

Anyone who is deliberately only installing Tutor will know what to do about this because they'll be familiar, and it's very likely they already have a config.yml file without those entries in the plugin list.

[xitij2000]

@xitij2000 Thanks for these updates. Image building succeeds, though it should be one of the commands that fails if an enabled plugin is missing, in my opinion. Actually, launch also gets off the ground right now. Can you double-check that things are actually failing for you where they need to be?

CI is also failing.

You noted to me over our internal ticket that this set of errors would cause a freshly installed tutor launch to fail due to installing only tutor leaving out plugins like mfe and indigo. However, I actually think this is preferable-- if someone accidentally installs only tutor and launches a brand new install, they're not following the tutorial closely and have almost certainly made a mistake.

Anyone who is deliberately only installing Tutor will know what to do about this because they'll be familiar, and it's very likely they already have a config.yml file without those entries in the plugin list.

Now command should run but produce an error at the end and return a error code. While this is not ideal, raising an error first means that you can't even run a command to fix the error, as was the case before.

This way if you apply a config change and that raises a plugin error it will raise immediately.

While I agree about the missing plugins raising an error by default it is a breaking change and I'd want some upstream feedback on this.