Refactor Runners, introduce Task class

[merelcht]

Description

Introduced the Task class, which encapsulates what is actually run in each of the runners to make the Runners code more readable.

I've always found the code in runners a bit hard to navigate. The (simplified) flow before my refactor for running a node was:

graph TD
    A[run/run_node in runner.py] --> B[_run in sequential_runner.py]
    A[run/run_node in runner.py] --> C[_run in thread_runner.py]
    A[run/run_node in runner.py] --> D[_run in parallel_runner.py]
    B -->  A[run/run_node in runner.py]
    C -->     A[run/run_node in runner.py]
    D -->     A[run/run_node in runner.py]
    A--> F[run in node.py]

Loading

Now it's:

graph TD
    A[run in runner.py] --> B[_run in sequential_runner.py]
    A[run in runner.py] --> C[_run in thread_runner.py]
    A[run in runner.py] --> D[_run in parallel_runner.py]
    B --> E[execute in task.py]
    C --> E[execute in task.py]
    D --> E[execute in task.py]
    E --> F[run in node.py]

Loading

Development notes

• Created Task which contains all that is needed to execute a Node.
• Created _release_datasets() to remove duplicated code across the runners.
• Moved all helper methods related to running a node from runner.py to the Task class.
• Moved the helper methods in parallel_runner.py to task.py
• Marked run_node() as deprecated, because it's replaced by Task.execute() and no longer called directly anywhere other than tests.

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

[merelcht]

This is needed because I refactored run_node in the runner to use Task and moved methods to Task as well. run_node isn't actually needed anymore, but removing it would be a breaking change. I could undo the changes to runner.py which removes the import from Task and then allows it to be imported inside the runner implementations again. The downside is that we'd have duplicated code in runner.py and task.py.

[deepyaman]

Is there a relevant issue for this? Nothing against the idea of introducing the "task" abstraction; just interested to better understand what motivates it.

[merelcht]

Is there a relevant issue for this? Nothing against the idea of introducing the "task" abstraction; just interested to better understand what motivates it.

I'll update the description when the PR is ready for review.

[noklam]

In general not much concern the refactoring looks simple enough and is a better abstraction than the previous one. I would like to run the benchmark once #4210 is ready to make sure we don't run into memory/perf issue.

Side note: I think the current Task is a bit weird with node/hook manager/catalog, but I understand it's necessary for keeping it non-breaking so it's all good, maybe something to revise when we are closer to 0.20.0

[ElenaKhaustova]

Looks much cleaner now, thank you @merelcht!

Left one minor suggestion.

[ElenaKhaustova]

Suggested change

	if load_counts[dataset] < 1 and dataset not in pipeline.inputs():
	if load_counts[dataset] and dataset not in pipeline.inputs():

[merelcht]

@ElenaKhaustova can you explain the suggestion of removing the check for the load_counts[dataset] to be smaller than 1?