Cancel event sources earlier #634
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
This is an attempt to address ansible#633 but it's very ham-fisted.
sc68cal
force-pushed
the
cancel_sources_graceful
branch
from
49f75ff to
3670e97
Compare
|
With this PR and the RabbitMQ event source plugin, you can see that the ruleset runs a playbook (that just runs pause for 30 seconds to simulate work being done), and stops pulling from the RabbitMQ event source, while also completing all the work that it already pulled from the event source before the shutdown event was processed. ---
- name: Rulebook for OpenStack TSF Controller
hosts: all
sources:
- ansible.eda.rabbitmq:
host: localhost
port: 5672
queue: "openstack-tsf-network-controller-{{GENERATION}}"
user: guest
password: guest
routing_key: 'platform.openstack.*.tsf.rendering.network.*'
exchange: "platform_requests"
- ansible.eda.webhook:
port: 5000
rules:
- name: "tsf network create rabbitmq event"
condition: event.body.event == 'createOpenStackNetwork'
actions:
- run_module:
name: pause
module_args:
seconds: 30
- name: "shutdown via webhook"
condition: event.payload.action == "shutdown"
action:
shutdown:
message: "Shutting down"
delay: 600I also confirm that there are still messages in the RabbitMQ queue, via the management plugin |
|
@sc68cal Would it make more sense if we write a function called cancel_all_sources which can be called when a shutdown is issued. We would have to store all the tasks in a global variable in addition to this and use that global variable in cancel_all_sources, we would have to check if the task is done before we call cancel on it.. This function can be added in the app.py file. We would also need a task.done check here |
Where would this new function be declared? The issue is that the coroutine for the
Generally, think global variables are harmful and should be avoided. It makes the code harder to follow. Plus, we'd have to use locks or flags from asyncio to coordinate access, which may not be worth doing.
I'm not sure what you mean here - because at least on the RabbitMQ plugin I've written, it has a coroutine that
I'm not sure why we would be checking for done here, because we are trying to cancel consuming from event sources, that are never |
sc68cal
commented
Jan 2, 2024
|
Some of the tests create a @@ -1045,7 +1045,7 @@ async def test_36_multiple_rulesets_both_fired():
):
await run_rulesets(
event_log,
- [],
+ [ruleset_queues[1][0].sources[0]],
ruleset_queues,
dict(),
"playbooks/inventory.yml",but |
|
This also changes some of the behaviors of the end to end tests. I think it's because of the ordering of the messages and when the shutdown message is sent, but I'm not 100% sure. |
tasks was a variable that contained the return from spawn_sources as well as the websocket feedback task. These are different things, and should not be passed into run_rulesets, since the feedback task is not an event source. tasks is now used as a list of all tasks, only for the asyncio.gather call
This is fixed by handling the feedback task differently in the |
|
Hi, Can I get some feedback on this pull request from the maintainers? I did a pretty deep dive on the internals of |
|
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.
This is an attempt to address #633 but it's very ham-fisted.