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

fix: validate that the "old" part of a reconfigured pair will get garbage collected #358

Open
wants to merge 4 commits into
base: transition-to-runkit
Choose a base branch
from

Conversation

doudou
Copy link
Member

@doudou doudou commented Apr 11, 2023

On top of #357

This commit is to essentially avoid the silent failure. With this change, Syskit will kill the old task, with the likely
effect of actually killing the main action. But better than blindly waiting (maybe ?)

I put it onto a separate pull request to get a bit more time with it before we merge, making sure there are no
unintended consequences, while #357 is, I believe, safe.

…ions

While the normal data structure does not generate this type of patterns,
they must be supported - it is generally speaking useful, and is already
used by e.g. the transformer to represent dynamic transformation producers.

The pattern obviously already triggered a bug since there was a specific
bug fix for a task context->task context dependency. The fix was however
too limited as it would not handle task context->composition.

This change implements a pass after deployment where we would find the
root components from the "old plan" that are not used in the "new plan",
and cut the dependency relations between these two. This should really
handle all cases.
…l deployment stages

finalize_deployed_tasks split between reused and new tasks, while
reconfigure_tasks_on_static_port_modification only had one
"deployed tasks". It was pretty confusing (even if the two get merged
at the end)
…bage collected

This makes explicit an internal error, and avoids that the system
blindly "waits" for the task to finally do what it should.
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