Polling sub tasks #243
Polling sub tasks #243
Conversation
adamruzicka
commented
Jul 24, 2017
•
adamruzicka
commented
- resuming
- tests
- tests for polling
- tests for polling + bulk
- tests for resume
|
|
||
| def wait_for_sub_plans(sub_plans) | ||
| increase_counts(sub_plans.count, 0) | ||
| if is_a?(::Dynflow::Action::WithBulkSubPlans) |
There was a problem hiding this comment.
We don't really need it, but it makes sense to start polling after all the sub plans were spawned.
|
|
||
| def recalculate_counts | ||
| total = sub_plans.count | ||
| @sub_plans = nil # TODO: |
There was a problem hiding this comment.
Planning to clean this TODOs, right? :)
There was a problem hiding this comment.
This is being fixed in #240 , should I pull the changes here as well?
There was a problem hiding this comment.
If they are dependent, having two commits in one PR, with the final set of changes would be the best.
There was a problem hiding this comment.
Both PRs should now contain commit fdf57ed with the common changes
test/action_test.rb
Outdated
| describe ::Dynflow::Action::WithPollingSubPlans do | ||
| include TestHelpers | ||
|
|
||
| let(:klok) { Dynflow::Testing::ManagedClock.new } |
There was a problem hiding this comment.
Any reason not to call it :clock?
There was a problem hiding this comment.
not really, will change it
|
Codewise looks good, waiting for the test of the tests to start testing this manually |
The old behavior was to cache the results of WithSubPlans#sub_plans, which was confusing when using filters. For example action.sub_plans(:state => :stopped) action.sub_plans(:state => 'non-existent state') would return all sub plans in :stopped state twice.
adamruzicka
force-pushed
the
polling-sub-tasks
branch
from
f9509c4
to
e95ad4c
Compare
|
Tested and works well. Thanks @adamruzicka. Please crate an issue for foreman_remote_execution to start using it and let's add it there. I think it's a prerequisity for http://projects.theforeman.org/issues/19937/ to work as expected (from the user point of view) |
