Add publisher to ActionAlias #5763

ubaumann · 2022-10-06T21:01:32Z

Add publisher to ActionAlias to get events for CUD Action Aliases.
This change enables DroidStorm (and other systems) to subscribe the events and get informed when a new Alias is created, updated or deleted.

Manual tested:

Start st2 (./tools/launchdev.sh startclean)
Start queue_consumer
Create and delete action-alias

Created and deleted action-alias

(virtualenv) ins@ubuntu:~/st2$ st2 action-alias create contrib/hello_st2/aliases/alias1.yaml
+----------------------+------------------------------+
| Property             | Value                        |
+----------------------+------------------------------+
| id                   | 633f3ceb2735066974631da4     |
| name                 | greet                        |
| pack                 | hello_st2                    |
| description          | Greet StackStorm             |
| ack                  |                              |
| action_ref           | hello_st2.greet              |
| enabled              | True                         |
| extra                |                              |
| formats              | [                            |
|                      |     "greet {{greeting}}"     |
|                      | ]                            |
| immutable_parameters |                              |
| metadata_file        |                              |
| ref                  | hello_st2.greet              |
| result               |                              |
| uid                  | action_alias:hello_st2:greet |
+----------------------+------------------------------+
(virtualenv) ins@ubuntu:~/st2$ st2 action-alias delete greet
Resource with id "greet" has been successfully deleted.

Queu consumer

(virtualenv) ins@ubuntu:~/st2$ ST2_CONFIG_PATH=conf/st2.dev.conf python tools/queue_consumer.py --exchange st2.actionalias
===================================================
Received message
message.properties:
{'application_headers': {},
 'content_encoding': 'binary',
 'content_type': 'application/x-python-serialize',
 'delivery_mode': 2,
 'priority': 0}
message.delivery_info:
{'consumer_tag': 'None1',
 'delivery_tag': 1,
 'exchange': 'st2.actionalias',
 'redelivered': False,
 'routing_key': 'create'}
body:
<ActionAliasDB: ActionAliasDB(ack={}, action_ref="hello_st2.greet", description="Greet StackStorm", enabled=True, extra={}, formats=['greet {{greeting}}'], id=633f3ceb2735066974631da4, immutable_parameters={}, metadata_file="", name="greet", pack="hello_st2", ref="hello_st2.greet", result={}, uid="action_alias:hello_st2:greet")>
===================================================
===================================================
Received message
message.properties:
{'application_headers': {},
 'content_encoding': 'binary',
 'content_type': 'application/x-python-serialize',
 'delivery_mode': 2,
 'priority': 0}
message.delivery_info:
{'consumer_tag': 'None1',
 'delivery_tag': 2,
 'exchange': 'st2.actionalias',
 'redelivered': False,
 'routing_key': 'delete'}
body:
<ActionAliasDB: ActionAliasDB(ack={}, action_ref="hello_st2.greet", description="Greet StackStorm", enabled=True, extra={}, formats=['greet {{greeting}}'], id=633f3ceb2735066974631da4, immutable_parameters={}, metadata_file="", name="greet", pack="hello_st2", ref="hello_st2.greet", result={}, uid="action_alias:hello_st2:greet")>
===================================================

Unitest are covering already testing the generic functions. There are no test testing the object specific events. Should I add some test?

Looking forward to the review and please let me know how I can improve the PR.

Pinging @cognifloyd because we talked already in Slack about the PR.

CLAassistant · 2022-10-06T21:01:40Z

All committers have signed the CLA.

cognifloyd

Awesome! I just found one more file we need to edit to make these events available in the event stream.

Would you please add the ActionAlias queue to this list:

st2/st2common/st2common/stream/listener.py

Lines 203 to 224 in 8496bb2

    
           return [ 
        
               consumer( 
        
                   queues=[STREAM_ANNOUNCEMENT_WORK_QUEUE], 
        
                   accept=["pickle"], 
        
                   callbacks=[self.processor()], 
        
               ), 
        
               consumer( 
        
                   queues=[STREAM_EXECUTION_ALL_WORK_QUEUE], 
        
                   accept=["pickle"], 
        
                   callbacks=[self.processor(ActionExecutionAPI)], 
        
               ), 
        
               consumer( 
        
                   queues=[STREAM_LIVEACTION_WORK_QUEUE], 
        
                   accept=["pickle"], 
        
                   callbacks=[self.processor(LiveActionAPI)], 
        
               ), 
        
               consumer( 
        
                   queues=[STREAM_EXECUTION_OUTPUT_QUEUE], 
        
                   accept=["pickle"], 
        
                   callbacks=[self.processor(ActionExecutionOutputAPI)], 
        
               ), 
        
           ]

Also, just a couple nitpicks.

Thank you so much for working on this!

We might have to wait a bit before merging this as I'm not sure if it can go in the 3.8 release.

st2common/st2common/transport/actionalias.py

CHANGELOG.rst

Co-authored-by: Jacob Floyd <[email protected]>

ubaumann · 2022-10-14T18:58:04Z

Awesome! I just found one more file we need to edit to make these events available in the event stream.

Would you please add the ActionAlias queue to this list:

st2/st2common/st2common/stream/listener.py

Lines 203 to 224 in 8496bb2

return [

consumer(

queues=[STREAM_ANNOUNCEMENT_WORK_QUEUE],

accept=["pickle"],

callbacks=[self.processor()],

),

consumer(

queues=[STREAM_EXECUTION_ALL_WORK_QUEUE],

accept=["pickle"],

callbacks=[self.processor(ActionExecutionAPI)],

),

consumer(

queues=[STREAM_LIVEACTION_WORK_QUEUE],

accept=["pickle"],

callbacks=[self.processor(LiveActionAPI)],

),

consumer(

queues=[STREAM_EXECUTION_OUTPUT_QUEUE],

accept=["pickle"],

callbacks=[self.processor(ActionExecutionOutputAPI)],

),

]

Thank you very much for finding this place. I totally missed it.

Also, just a couple nitpicks.

Thank you for reviewing the PR

We might have to wait a bit before merging this as I'm not sure if it can go in the 3.8 release.

That's totally fine. I was lately a little busy but it was really nice to work on ST2 and I am planning to keep sticking around. So just let me know if I should rebase the branch or anything else.

cognifloyd

Woohoo! Awesome. Thanks for working on this.

This needs 1 more approval, and @nzlosh needs to decide if it can land in 3.8 or if it needs to wait till after we finish cutting that release.

amanda11

Minor comment on changelog, and I'm wondering if its possible to add some unit-tests for the changes.

CHANGELOG.rst

cognifloyd · 2022-12-04T01:17:43Z

I just went through a bunch of tests to see which tests deal somehow with publishing messages on the queue. In most cases, it is mocking or ignoring the message queue publishing.

Based on that, I think we could add some tests in st2common/tests/unit/test_transport.py. The new test(s) can use the QueueConsumer class similar to how TransportUtilsTestCase.test_publish_compression() does it to test that messages get published to the right queue. Instead of using the dummy Exchange+Queue+PoolPublisher, we would use:

st2common.transport.actionalias.ACTIONALIAS_XCHG
st2common.transport.actionalias.get_queue()
I think all we need to use this safely (safe for parallel tests that touch RabbitMQ)
is a test-specific queue_name, and then:
get_queue(name=queue_name, routing_key="#", exclusive=True, auto_delete=True)
st2common.transport.actionalias.ActionAliasPublisher().

I don't see how we can test this without effectively testing kombu+RabbitMQ, since much of the added code added in this PR deals with wiring up the exchange+queue, setting it up for publishing.

We could also add a db test similar to the test_db_*.py files that mocks ActionAliasPublisher and makes sure that publish() gets called when we create/update/delete (but not read) with ActionAliasDB.

@amanda11 Does that sound like what you'd like to see? Would tests like this have value?

Here are my raw notes:

uses liveaction and workflow publisher mocks from st2tests.mocks (see below)
- https://github.com/StackStorm/st2/blob/master/st2common/tests/unit/services/test_workflow*.py
mocks PoolPublisher.publish to ignore publishing
checks that LiveActionPublisher.publish_state gets called and mocks PoolPublisher.publish to ignore CRUD publishing
- https://github.com/StackStorm/st2/blob/master/st2common/tests/unit/test_action_db_utils.py
tests st2common.transport.connection_retry_wrapper
- https://github.com/StackStorm/st2/blob/master/st2common/tests/unit/test_connection_retry_wrapper.py
tests st2common.transport.publishers.StatePublisherMixin
- https://github.com/StackStorm/st2/blob/master/st2common/tests/unit/test_state_publisher.py
tests st2common.stream.listener with mocked transport messages
- https://github.com/StackStorm/st2/blob/master/st2common/tests/unit/test_stream_generator.py
tests st2common.transport misc
- https://github.com/StackStorm/st2/blob/master/st2common/tests/unit/test_transport.py
  - TransportUtilsTestCase.test_publish_compression:
    tests st2common.transport.publishers.PoolPublisher.publish()
    with various [messaging].compression st2.conf settings
  - TransportUtilsTestCase.test_get_ssl_kwargs:
    tests st2common.transport.utils._get_ssl_kwargs()
tests listening with SSL settings
- https://github.com/StackStorm/st2/blob/master/st2common/tests/integration/test_rabbitmq_ssl_listener.py

mock publishers in:

rush-skills · 2023-02-03T12:23:38Z

@cognifloyd @ubaumann Is this still WIP?

cognifloyd · 2023-02-03T14:15:51Z

I would love to see this merged. I'm satisfied with this without the tests. But, @amanda11 mentioned adding tests, so I tried to figure out what tests we could add. @rush-skills wdyt? Does this need tests?

amanda11

I'm happy to merge as is, without the extra tests. I'd rather it be in, than wait months for tests...

ubaumann added 3 commits October 3, 2022 22:21

Add publischer to ActionAlias

7ff07b2

Add ActionAlias Exchange to bootstrap_utils.py

c06cdbe

Update CHANGELOG.rst

cd2ba8a

pull-request-size bot added the size/M PR that changes 30-99 lines. Good size to review. label Oct 6, 2022

cognifloyd added action aliases feature chatops labels Oct 9, 2022

cognifloyd requested changes Oct 9, 2022

View reviewed changes

st2common/st2common/transport/actionalias.py Outdated Show resolved Hide resolved

CHANGELOG.rst Outdated Show resolved Hide resolved

cognifloyd added the hacktoberfest-accepted label Oct 9, 2022

ubaumann and others added 3 commits October 14, 2022 20:52

Add ActionAlias queue to stream listener

2a928d9

Fix Copyright year

abbbd65

Co-authored-by: Jacob Floyd <[email protected]>

Update CHANGELOG.rst

b0ca413

Co-authored-by: Jacob Floyd <[email protected]>

ubaumann requested a review from cognifloyd October 14, 2022 18:59

cognifloyd approved these changes Oct 14, 2022

View reviewed changes

cognifloyd requested review from nzlosh, m4dcoder and a team October 14, 2022 22:58

cognifloyd changed the title ~~Add publischer to ActionAlias~~ Add publisher to ActionAlias Oct 14, 2022

cognifloyd requested a review from winem November 3, 2022 05:08

cognifloyd added this to the 3.9.0 milestone Nov 3, 2022

Merge branch 'master' into action_alias_publisher

94c489c

cognifloyd requested review from amanda11 and rush-skills November 24, 2022 06:02

amanda11 reviewed Dec 1, 2022

View reviewed changes

CHANGELOG.rst Outdated Show resolved Hide resolved

cognifloyd requested a review from amanda11 December 1, 2022 17:36

cognifloyd added 2 commits December 3, 2022 16:46

Merge branch 'master' into action_alias_publisher

e4e47f4

Move changelog entry

6c81f4f

amanda11 approved these changes Feb 3, 2023

View reviewed changes

Merge branch 'master' into action_alias_publisher

76891f3

cognifloyd enabled auto-merge February 3, 2023 22:28

cognifloyd merged commit 70a8637 into StackStorm:master Feb 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add publisher to ActionAlias #5763

Add publisher to ActionAlias #5763

ubaumann commented Oct 6, 2022 •

edited by cognifloyd

Loading

CLAassistant commented Oct 6, 2022 •

edited

Loading

cognifloyd left a comment

ubaumann commented Oct 14, 2022

cognifloyd left a comment

amanda11 left a comment

cognifloyd commented Dec 4, 2022 •

edited

Loading

rush-skills commented Feb 3, 2023

cognifloyd commented Feb 3, 2023

amanda11 left a comment

	return [
	consumer(
	queues=[STREAM_ANNOUNCEMENT_WORK_QUEUE],
	accept=["pickle"],
	callbacks=[self.processor()],
	),
	consumer(
	queues=[STREAM_EXECUTION_ALL_WORK_QUEUE],
	accept=["pickle"],
	callbacks=[self.processor(ActionExecutionAPI)],
	),
	consumer(
	queues=[STREAM_LIVEACTION_WORK_QUEUE],
	accept=["pickle"],
	callbacks=[self.processor(LiveActionAPI)],
	),
	consumer(
	queues=[STREAM_EXECUTION_OUTPUT_QUEUE],
	accept=["pickle"],
	callbacks=[self.processor(ActionExecutionOutputAPI)],
	),
	]

Add publisher to ActionAlias #5763

Add publisher to ActionAlias #5763

Conversation

ubaumann commented Oct 6, 2022 • edited by cognifloyd Loading

CLAassistant commented Oct 6, 2022 • edited Loading

cognifloyd left a comment

Choose a reason for hiding this comment

ubaumann commented Oct 14, 2022

cognifloyd left a comment

Choose a reason for hiding this comment

amanda11 left a comment

Choose a reason for hiding this comment

cognifloyd commented Dec 4, 2022 • edited Loading

rush-skills commented Feb 3, 2023

cognifloyd commented Feb 3, 2023

amanda11 left a comment

Choose a reason for hiding this comment

ubaumann commented Oct 6, 2022 •

edited by cognifloyd

Loading

CLAassistant commented Oct 6, 2022 •

edited

Loading

cognifloyd commented Dec 4, 2022 •

edited

Loading