feat: early deploy at network generation #445
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wvmcastro
force-pushed
the
early-deploy-on-generation
branch
5 times, most recently
from
309e84c to
396f3b5
Compare
wvmcastro
force-pushed
the
early-deploy-on-generation
branch
from
396f3b5 to
d7d69a0
Compare
wvmcastro
force-pushed
the
early-deploy-on-generation
branch
from
d7d69a0 to
b25b027
Compare
wvmcastro
force-pushed
the
early-deploy-on-generation
branch
from
6907422 to
1c41f2b
Compare
wvmcastro
changed the title
doudou
reviewed
Jan 29, 2025
wvmcastro
force-pushed
the
early-deploy-on-generation
branch
3 times, most recently
from
bc98f43 to
a5e3faf
Compare
wvmcastro
force-pushed
the
early-deploy-on-generation
branch
2 times, most recently
from
03d9cfe to
d999c66
Compare
necessary for sharing data with the public api that takes no arguments
propagate it from engine to system network generator
wvmcastro
force-pushed
the
early-deploy-on-generation
branch
from
d999c66 to
5d3caa1
Compare
jhonasiv
reviewed
Feb 3, 2025
it actually tests for instantiation, not for deployment
doudou
reviewed
Feb 6, 2025
the test was "succeding" because can_merge? was returning false, and not because both execution agents were non nil, as the test intended
In order to it be possible, I had to change merge_solver.merge_identical_tasks call to merge_solver.merge_compositions at Engine#finalize_deployed_tasks. With merged previously because all had non nil execution agents, but now when this rule changes with early_deploy and the merge solver would attempt to merge tasks contexts because their agents are non nil. It particulary breaks when for some reason the current task context cannot be deployed by its transaction proxy counter part as in "ensures that the old task gets garbage collected when child of another still useful task test"
when early deploying, a non deployed task can't be merged with a deployed one, because the basic assumption is that all tasks should have a deployment'
verifying it earlier led throwing deploying errors where there were primary instantiation errors
when early deploying the assumption changes to "every task should have an execution agent" so merging one that does with one that doesn't would be wrong. Note that when not early_deploying the merge criteria is exactly the opposite, two task contexts should be merged when just one of them has an execution agent
because all validate_* paramenters are passed to generate so far
wvmcastro
force-pushed
the
early-deploy-on-generation
branch
from
f3236cc to
d726d8e
Compare
move network generation exception to helpers
now it gives global context information
doudou
reviewed
Feb 12, 2025
doudou
reviewed
Feb 12, 2025
doudou
changed the title
jhonasiv
approved these changes
Feb 13, 2025
jhonasiv
reviewed
Feb 13, 2025
jhonasiv
reviewed
Feb 13, 2025
doudou
reviewed
Feb 14, 2025
lib/syskit/exceptions.rb
Outdated
Comment on lines
479
to
480
| agent = tasks.first.execution_agent | ||
| deployment_m = agent.deployed_orogen_model_by_name(orocos_name) |
There was a problem hiding this comment.
You need to extract this information in the constructor. There is no guarantee that the plan will be the way it was once pretty_print is called
Co-authored-by: jhonasiv <[email protected]>
Co-authored-by: jhonasiv <[email protected]>
The task->agent relation is registered in the plan graph and it changes between exception raise and the moment it is printed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On top of:
Depends on:
Opening this PR for getting a CI run with test failures and an early request for comments