Add option to raise errors during find_pipelines

[deepyaman]

Description

Resolves #2910

Development notes

Developer Certificate of Origin

We need all contributions to comply with the Developer Certificate of Origin (DCO). All commits must be signed off by including a Signed-off-by line in the commit message. See our wiki for guidance.

If your PR is blocked due to unsigned commits, then you must follow the instructions under "Rebase the branch" on the GitHub Checks page for your PR. This will retroactively add the sign-off to all unsigned commits and allow the DCO check to pass.

Checklist

• Read the contributing guidelines
• Signed off each commit with a Developer Certificate of Origin (DCO)
• Opened this PR as a 'Draft Pull Request' if it is work-in-progress
• Updated the documentation to reflect the code changes
• Added a description of this change in the RELEASE.md file
• Added tests to cover my changes
• Checked if this change will affect Kedro-Viz, and if so, communicated that with the Viz team

[noklam]

Implementation wise looks fine. I think we should update the relevant documentation as well. I was wondering should we also update the starter to add the default to make this more discoverable.

pipelines = find_pipelines(raise_errors=False)

[deepyaman]

Implementation wise looks fine. I think we should update the relevant documentation as well. I was wondering should we also update the starter to add the default to make this more discoverable.

pipelines = find_pipelines(raise_errors=False)

Good point! See 71e0f64 for documentation added in response.

[ElenaKhaustova]

The implementation looks good to me!

My only question is whether we should keep the default value raise_errors=False rather than raise_errors=True. If I understood correctly, the feature was initially designed for developers, so any half-developed pipeline won't prevent another pipeline from running. However, general users find debugging hard if an issue occurs and the pipeline is still running. So we may consider making raise_errors=True the default behaviour, as the chance that developers are aware of this feature and the flag is higher than that of general users.

[deepyaman]

My only question is whether we should keep the default value raise_errors=False rather than raise_errors=True. If I understood correctly, the feature was initially designed for developers, so any half-developed pipeline won't prevent another pipeline from running. However, general users find debugging hard if an issue occurs and the pipeline is still running. So we may consider making raise_errors=True the default behaviour, as the chance that developers are aware of this feature and the flag is higher than that of general users.

I'm OK with that, if that's the prevailing user feedback. @astrojuanlu @merelcht any opinion?

Also, I don't personally feel that behavior change is "breaking" (it's pretty easy to update your registry, and hopefully you're not running with a broken pipeline already in production), but would be happy to get others' view.

[merelcht]

My only question is whether we should keep the default value raise_errors=False rather than raise_errors=True. If I understood correctly, the feature was initially designed for developers, so any half-developed pipeline won't prevent another pipeline from running. However, general users find debugging hard if an issue occurs and the pipeline is still running. So we may consider making raise_errors=True the default behaviour, as the chance that developers are aware of this feature and the flag is higher than that of general users.

I'm OK with that, if that's the prevailing user feedback. @astrojuanlu @merelcht any opinion?

Also, I don't personally feel that behavior change is "breaking" (it's pretty easy to update your registry, and hopefully you're not running with a broken pipeline already in production), but would be happy to get others' view.

I think it's a good call to make the default raise_errors=True. We have indeed heard that users struggle with debugging, so raising errors explicitly by default will hopefully help.

[astrojuanlu]

Without a full understanding of #2401, I'd be wary of setting raise_errors=True on a micro release. We want to take a broader look at what kind of errors people find while debugging.

My recommendation would be to introduce the option but not change the behavior, so that we can take a more comprehensive look soon. That's also the safer option anyway in terms of backwards compatibility.

[ElenaKhaustova]

Approving the PR not to block it, since the implementation looks good to me.

Agree to decide on making the raise_errors=True later on.

[noklam]

Thanks, a bit late to the discussion. I agree we should not change the behavior in a micro release.

In principle I like a default=True, but with concern about discoverability and the ergonomics of this. I imagine this is like a switch that you may need to turn on & off from time to time, going into pipeline_registry.py to do so feel inconsistent with other CLI options etc.