Skip to content

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Apr 11, 2025

Signed-off-by: Philippe Martin [email protected]

What does this PR do?

Add a TaskRunner class to separate task logic and implementation

Screenshot / video of UI

N/A

What issues does this PR fix or reference?

Related to #2628

How to test this PR?

Start recipes and check everything works as before

Signed-off-by: Philippe Martin <[email protected]>
@feloy feloy requested review from benoitf, jeffmaury and a team as code owners April 11, 2025 10:55
@feloy feloy requested review from cdrage, deboer-tim and axel7083 April 11, 2025 10:55
Copy link
Collaborator

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@feloy feloy requested a review from axel7083 April 11, 2025 12:28
Signed-off-by: Philippe Martin <[email protected]>

export class ApplicationManager extends Publisher<ApplicationState[]> implements Disposable {
#applications: ApplicationRegistry<ApplicationState>;
protectTasks: Set<string> = new Set();
#disposables: Disposable[];
#taskRunner: TaskRunner;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if task runner is stored like that it should be disposable, and be disposed if the class is disposed, cancelling any pending tasks it is managing ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be a good enhancement, but as this is only a refactoring, I don't want to change any behaviour

@feloy feloy merged commit ef19032 into containers:main Apr 11, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants