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

Refactor registration of commands #512

Merged
merged 2 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ TBD
- Updated SystemClient to utilize the local Garden name for default Namespace if none can be determined
- Updated default Garden version to `UNKNOWN`
- Updated `get_current_request_read_only` to support sub threads calling where current_request is not populated
- Refactored Publishclient for Registering and Unregistering commands input values

3.27.2
------
Expand Down
82 changes: 46 additions & 36 deletions brewtils/rest/publish_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from brewtils.errors import BrewtilsException
from brewtils.models import Event, Events, Request, Subscriber, Topic
from brewtils.rest.easy_client import EasyClient
from brewtils.schema_parser import SchemaParser


class PublishClient(object):
Expand Down Expand Up @@ -64,6 +65,7 @@ class PublishClient(object):
def __init__(self, *args, **kwargs):
self._logger = logging.getLogger(__name__)
self._easy_client = EasyClient(*args, **kwargs)
self._schema_parser = SchemaParser()

def publish(
self,
Expand Down Expand Up @@ -156,13 +158,16 @@ def _get_parent_for_request(self):

return Request(id=str(parent.id))

def register_command(self, topic: str, cmd) -> Topic:
def register_command(
self, topic_name: str, cmd_name: str = None, cmd_func=None
) -> Topic:
"""Register a command to subscribe to the topic provided. The subscriber is
marked as GENERATED, and will be pruned after the system shuts down

Args:
topic (str): Topic for the command to subscribe to
cmd (str, function): Command to register
topic_name (str): Topic for the command to subscribe to
cmd_name (str): Command to register
cmd_func (function): Command to register

Raises:
BrewtilsException: If function is provided, it must be an annotated
Expand All @@ -186,38 +191,42 @@ def register_command(self, topic: str, cmd) -> Topic:
)
)

if isinstance(cmd, str):
command_name = cmd
else:
if not hasattr(cmd, "_command"):
if not cmd_name:
if not hasattr(cmd_func, "_command"):
raise BrewtilsException(
(
"Attempted to register command "
f"{getattr(cmd, '__name__', 'MISSING FUNC NAME')} "
f"{getattr(cmd_func, '__name__', 'MISSING FUNC NAME')} "
"that is not an annotated command"
)
)
command_name = cmd._command.name
cmd_name = cmd_func._command.name

return self._easy_client.update_topic(
topic_name=topic,
add=Subscriber(
garden=brewtils.plugin.CONFIG.garden,
namespace=brewtils.plugin.CONFIG.namespace,
system=brewtils.plugin.CONFIG.name,
version=brewtils.plugin.CONFIG.version,
instance=brewtils.plugin.CONFIG.instance_name,
command=command_name,
subscriber_type="GENERATED",
topic_name=topic_name,
add=self._schema_parser.serialize_subscriber(
Subscriber(
garden=brewtils.plugin.CONFIG.garden,
namespace=brewtils.plugin.CONFIG.namespace,
system=brewtils.plugin.CONFIG.name,
version=brewtils.plugin.CONFIG.version,
instance=brewtils.plugin.CONFIG.instance_name,
command=cmd_name,
subscriber_type="GENERATED",
),
to_string=False,
),
)

def unregister_command(self, topic: str, cmd) -> Topic:
def unregister_command(
self, topic_name: str, cmd_name: str = None, cmd_func=None
) -> Topic:
"""Unregister a command to subscribe to the topic provided.

Args:
topic (str): Topic for the command to subscribe to
cmd (str, function): Command to unregister
topic_name (str): Topic for the command to subscribe to
cmd_name (str): Command to unregister
cmd_func (function): Command to unregister

Raises:
BrewtilsException: If function is provided, it must be
Expand All @@ -240,28 +249,29 @@ def unregister_command(self, topic: str, cmd) -> Topic:
)
)

if isinstance(cmd, str):
command_name = cmd
else:
if not hasattr(cmd, "_command"):
if not cmd_name:
if not hasattr(cmd_func, "_command"):
raise BrewtilsException(
(
"Attempted to register command "
f"{getattr(cmd, '__name__', 'MISSING FUNC NAME')} "
f"{getattr(cmd_func, '__name__', 'MISSING FUNC NAME')} "
"that is not an annotated command"
)
)
command_name = cmd._command.name
cmd_name = cmd_func._command.name

return self._easy_client.update_topic(
topic_name=topic,
remove=Subscriber(
garden=brewtils.plugin.CONFIG.garden,
namespace=brewtils.plugin.CONFIG.namespace,
system=brewtils.plugin.CONFIG.name,
version=brewtils.plugin.CONFIG.version,
instance=brewtils.plugin.CONFIG.instance_name,
command=command_name,
subscriber_type="GENERATED",
topic_name=topic_name,
remove=self._schema_parser.serialize_subscriber(
Subscriber(
garden=brewtils.plugin.CONFIG.garden,
namespace=brewtils.plugin.CONFIG.namespace,
system=brewtils.plugin.CONFIG.name,
version=brewtils.plugin.CONFIG.version,
instance=brewtils.plugin.CONFIG.instance_name,
command=cmd_name,
subscriber_type="GENERATED",
),
to_string=False,
),
)
14 changes: 7 additions & 7 deletions test/rest/publish_client_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,14 @@ def test_register_command(self, client, easy_client):
def _cmd(self, x):
return x

client.register_command("topic", _cmd)
client.register_command(topic_name="topic", cmd_func=_cmd)

easy_client.update_topic.assert_called()

def test_register_command_string(self, client, easy_client):
self.setup_config()

client.register_command("topic", "command")
client.register_command(topic_name="topic", cmd_name="command")

easy_client.update_topic.assert_called()

Expand All @@ -121,7 +121,7 @@ def test_unregister_command(self, client, easy_client):
def _cmd(self, x):
return x

client.unregister_command("topic", _cmd)
client.unregister_command(topic_name="topic", cmd_func=_cmd)

easy_client.update_topic.assert_called()

Expand All @@ -139,7 +139,7 @@ def _cmd(self, x):
return x

with pytest.raises(BrewtilsException):
client.register_command("topic", _cmd)
client.register_command(topic_name="topic", cmd_func=_cmd)

def test_unregister_command_non_annotated(self, client):
self.setup_config()
Expand All @@ -148,7 +148,7 @@ def _cmd(self, x):
return x

with pytest.raises(BrewtilsException):
client.unregister_command("topic", _cmd)
client.unregister_command(topic_name="topic", cmd_func=_cmd)

def test_register_command_no_config(self, client):

Expand All @@ -157,7 +157,7 @@ def _cmd(self, x):
return x

with pytest.raises(BrewtilsException):
client.register_command("topic", _cmd)
client.register_command(topic_name="topic", cmd_func=_cmd)

def test_unregister_command_no_config(self, client):

Expand All @@ -166,4 +166,4 @@ def _cmd(self, x):
return x

with pytest.raises(BrewtilsException):
client.unregister_command("topic", _cmd)
client.unregister_command(topic_name="topic", cmd_func=_cmd)
Loading