From e3d52369a09681e765aceac95e4fdc17c3f9c175 Mon Sep 17 00:00:00 2001 From: Jonah Kraushaar Date: Wed, 5 Aug 2020 01:03:59 +0200 Subject: [PATCH 01/12] Fixing example --- examples/async_advice_app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/async_advice_app.py b/examples/async_advice_app.py index 73dcaea..255280b 100644 --- a/examples/async_advice_app.py +++ b/examples/async_advice_app.py @@ -16,7 +16,7 @@ None of the authors, contributors, administrators, or anyone else connected with Rhasspy_Hermes_App, in any way whatsoever, can be responsible for your use of the api endpoint. """ -URL = 'https://api.advicslip.com/advice' +URL = 'https://api.adviceslip.com/advice' @app.on_intent("GetAdvice") async def get_advice(intent: NluIntent): From 1e0ca9f741bb34e107ec333aa3770d6539df5286 Mon Sep 17 00:00:00 2001 From: Jonah Kraushaar Date: Wed, 5 Aug 2020 01:04:33 +0200 Subject: [PATCH 02/12] Simple Option to enable keyword arguments --- rhasspyhermes_app/__init__.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rhasspyhermes_app/__init__.py b/rhasspyhermes_app/__init__.py index 83c07df..5a85a7b 100644 --- a/rhasspyhermes_app/__init__.py +++ b/rhasspyhermes_app/__init__.py @@ -85,6 +85,7 @@ def __init__( name: str, parser: Optional[argparse.ArgumentParser] = None, mqtt_client: Optional[mqtt.Client] = None, + **kwargs ): """Initialize the Rhasspy Hermes app. @@ -106,6 +107,11 @@ def __init__( # Parse command-line arguments self.args = parser.parse_args() + # Option to set all parameters as keyword arguments + args_dict = vars(self.args) + for key, value in kwargs.items(): + args_dict[key] = value + # Set up logging hermes_cli.setup_logging(self.args) _LOGGER.debug(self.args) From e17e20cb35012d1070e09ab8402f4dca47f2bcf2 Mon Sep 17 00:00:00 2001 From: Jonah Kraushaar Date: Wed, 5 Aug 2020 01:15:15 +0200 Subject: [PATCH 03/12] Added docs change for keyword args --- docs/usage.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/usage.rst b/docs/usage.rst index 8205d48..dc76439 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -31,6 +31,9 @@ Try the example app `time_app.py`_ with the ``--help`` flag to see what settings .. _`time_app.py`: https://github.com/rhasspy/rhasspy-hermes-app/blob/master/examples/time_app.py +You can pass all the settings as keyword arguments inside the constructor aswell: +:meth:`HermesApp("ExampleApp", host = "192.168.178.123", port = 12183)` + ******* Asyncio ******* @@ -65,7 +68,7 @@ If the API of this library changes, your app possibly stops working when it upda .. code-block:: - rhasspy-hermes-app==0.2.0 + rhasspy-hermes-app==1.0.0 This way your app keeps working when the Rhasspy Hermes App adds incompatible changes in a new version. From e4d9b4ddeda21f3aed008900e868740f10a60032 Mon Sep 17 00:00:00 2001 From: Jonah Kraushaar Date: Wed, 26 Aug 2020 22:12:26 +0200 Subject: [PATCH 04/12] Raising CLI Args priority --- rhasspyhermes_app/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rhasspyhermes_app/__init__.py b/rhasspyhermes_app/__init__.py index 5a85a7b..3bac6d3 100644 --- a/rhasspyhermes_app/__init__.py +++ b/rhasspyhermes_app/__init__.py @@ -107,10 +107,10 @@ def __init__( # Parse command-line arguments self.args = parser.parse_args() - # Option to set all parameters as keyword arguments + # Option to set all parameters as keyword arguments !!! CLI arguments are rated higher priority args_dict = vars(self.args) for key, value in kwargs.items(): - args_dict[key] = value + args_dict[key] = args_dict[key] if key in args_dict else value # Set up logging hermes_cli.setup_logging(self.args) From 5e1fac008c5896ba72fac13f7d294d5a98cd75af Mon Sep 17 00:00:00 2001 From: JonahKr <38377070+JonahKr@users.noreply.github.com> Date: Wed, 26 Aug 2020 22:44:41 +0200 Subject: [PATCH 05/12] Why 3 if it could just be one line... and better python --- rhasspyhermes_app/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/rhasspyhermes_app/__init__.py b/rhasspyhermes_app/__init__.py index 3bac6d3..5e03904 100644 --- a/rhasspyhermes_app/__init__.py +++ b/rhasspyhermes_app/__init__.py @@ -108,9 +108,7 @@ def __init__( self.args = parser.parse_args() # Option to set all parameters as keyword arguments !!! CLI arguments are rated higher priority - args_dict = vars(self.args) - for key, value in kwargs.items(): - args_dict[key] = args_dict[key] if key in args_dict else value + args_dict = {**kwargs, **vars(self.args)} # Set up logging hermes_cli.setup_logging(self.args) From 8aa91ea2268a0070fef93723db560fa711e10133 Mon Sep 17 00:00:00 2001 From: Jonah Kraushaar Date: Wed, 26 Aug 2020 22:52:34 +0200 Subject: [PATCH 06/12] Fixing args namespace dictionary replacement --- rhasspyhermes_app/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rhasspyhermes_app/__init__.py b/rhasspyhermes_app/__init__.py index 5e03904..3123231 100644 --- a/rhasspyhermes_app/__init__.py +++ b/rhasspyhermes_app/__init__.py @@ -108,7 +108,8 @@ def __init__( self.args = parser.parse_args() # Option to set all parameters as keyword arguments !!! CLI arguments are rated higher priority - args_dict = {**kwargs, **vars(self.args)} + args_dict = vars(self.args) + args_dict = {**kwargs, **args_dict} # Set up logging hermes_cli.setup_logging(self.args) From 6f3cc3a22077046daf897dfadf4047193cdcbc7b Mon Sep 17 00:00:00 2001 From: Koen Vervloesem Date: Thu, 27 Aug 2020 10:03:03 +0200 Subject: [PATCH 07/12] Add test for argument parsing --- tests/test_arguments.py | 97 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 tests/test_arguments.py diff --git a/tests/test_arguments.py b/tests/test_arguments.py new file mode 100644 index 0000000..3007412 --- /dev/null +++ b/tests/test_arguments.py @@ -0,0 +1,97 @@ +"""Tests for HermesApp arguments.""" +import argparse +import pytest +import sys + +import rhasspyhermes.cli as hermes_cli +from rhasspyhermes_app import HermesApp + + +def test_default_arguments(mocker): + """Test whether default arguments are set up correctly in a HermesApp object.""" + app = HermesApp("Test default arguments", mqtt_client=mocker.MagicMock()) + + assert app.args.host == "localhost" + assert app.args.port == 1883 + assert app.args.tls == False + assert app.args.username is None + assert app.args.password is None + + +def test_arguments_from_cli(mocker): + """Test whether arguments from the command line are set up correctly in a HermesApp object.""" + mocker.patch( + "sys.argv", + [ + "rhasspy-hermes-app-test", + "--host", + "rhasspy.home", + "--port", + "8883", + "--tls", + "--username", + "rhasspy-hermes-app", + "--password", + "test", + ], + ) + app = HermesApp("Test arguments in init", mqtt_client=mocker.MagicMock()) + + assert app.args.host == "rhasspy.home" + assert app.args.port == 8883 + assert app.args.tls == True + assert app.args.username == "rhasspy-hermes-app" + assert app.args.password == "test" + + +def test_arguments_in_init(mocker): + """Test whether arguments are set up correctly while initializing a HermesApp object.""" + app = HermesApp( + "Test arguments in init", + mqtt_client=mocker.MagicMock(), + host="rhasspy.home", + port=8883, + tls=True, + username="rhasspy-hermes-app", + password="test", + ) + + assert app.args.host == "rhasspy.home" + assert app.args.port == 8883 + assert app.args.tls == True + assert app.args.username == "rhasspy-hermes-app" + assert app.args.password == "test" + + +def test_if_cli_arguments_overwrite_init_arguments(mocker): + """Test whether arguments from the command line overwrite arguments to a HermesApp object.""" + mocker.patch( + "sys.argv", + [ + "rhasspy-hermes-app-test", + "--host", + "rhasspy.home", + "--port", + "8883", + "--tls", + "--username", + "rhasspy-hermes-app", + "--password", + "test", + ], + ) + app = HermesApp( + "Test arguments in init", + mqtt_client=mocker.MagicMock(), + host="rhasspy.local", + port=1883, + tls=False, + username="rhasspy-hermes-app-test", + password="covfefe", + ) + + assert app.args.host == "rhasspy.home" + assert app.args.port == 8883 + assert app.args.tls == True + assert app.args.username == "rhasspy-hermes-app" + assert app.args.password == "test" From bbf03dcddc34e621be81014f6d817d931c6e709c Mon Sep 17 00:00:00 2001 From: Koen Vervloesem Date: Thu, 27 Aug 2020 18:49:55 +0200 Subject: [PATCH 08/12] Fix precedence with default CLI arguments --- rhasspyhermes_app/__init__.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/rhasspyhermes_app/__init__.py b/rhasspyhermes_app/__init__.py index 3123231..7fd8721 100644 --- a/rhasspyhermes_app/__init__.py +++ b/rhasspyhermes_app/__init__.py @@ -105,11 +105,15 @@ def __init__( hermes_cli.add_hermes_args(parser) # Parse command-line arguments - self.args = parser.parse_args() - - # Option to set all parameters as keyword arguments !!! CLI arguments are rated higher priority - args_dict = vars(self.args) - args_dict = {**kwargs, **args_dict} + # Command-line arguments take precedence over the arguments of the HermesApp.__init__ + args_dict = vars(parser.parse_args()) + default_args_dict = vars(parser.parse_args([])) + # Remove the arguments which have their default values + non_default_args_dict = dict(set(args_dict.items()) - set(default_args_dict.items())) + # Let the non-default arguments take precedence over the object arguments + args = {**kwargs, **non_default_args_dict} + # Merge these back with the original arguments, taking into account precedence + self.args = argparse.Namespace(**{**args_dict, **args}) # Set up logging hermes_cli.setup_logging(self.args) From 2a35f054e1d4b5c20db80cd546c21e1fb8151c7d Mon Sep 17 00:00:00 2001 From: Koen Vervloesem Date: Thu, 27 Aug 2020 19:50:40 +0200 Subject: [PATCH 09/12] Cover explicit passing of default argument in test --- rhasspyhermes_app/__init__.py | 6 +++++- tests/test_arguments.py | 12 ++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/rhasspyhermes_app/__init__.py b/rhasspyhermes_app/__init__.py index 7fd8721..6c2deb8 100644 --- a/rhasspyhermes_app/__init__.py +++ b/rhasspyhermes_app/__init__.py @@ -109,7 +109,9 @@ def __init__( args_dict = vars(parser.parse_args()) default_args_dict = vars(parser.parse_args([])) # Remove the arguments which have their default values - non_default_args_dict = dict(set(args_dict.items()) - set(default_args_dict.items())) + non_default_args_dict = dict( + set(args_dict.items()) - set(default_args_dict.items()) + ) # Let the non-default arguments take precedence over the object arguments args = {**kwargs, **non_default_args_dict} # Merge these back with the original arguments, taking into account precedence @@ -124,6 +126,7 @@ def __init__( mqtt_client = mqtt.Client() # Initialize HermesClient + # pylint: disable=no-member super().__init__(name, mqtt_client, site_ids=self.args.site_id) self._callbacks_hotword: List[Callable[[HotwordDetected], Awaitable[None]]] = [] @@ -602,6 +605,7 @@ def run(self): self._subscribe_callbacks() # Try to connect + # pylint: disable=no-member _LOGGER.debug("Connecting to %s:%s", self.args.host, self.args.port) hermes_cli.connect(self.mqtt_client, self.args) self.mqtt_client.loop_start() diff --git a/tests/test_arguments.py b/tests/test_arguments.py index 3007412..da7a0f7 100644 --- a/tests/test_arguments.py +++ b/tests/test_arguments.py @@ -3,7 +3,6 @@ import pytest import sys -import rhasspyhermes.cli as hermes_cli from rhasspyhermes_app import HermesApp @@ -72,26 +71,23 @@ def test_if_cli_arguments_overwrite_init_arguments(mocker): "--host", "rhasspy.home", "--port", - "8883", - "--tls", + "1883", "--username", "rhasspy-hermes-app", "--password", - "test", + "test" ], ) app = HermesApp( "Test arguments in init", mqtt_client=mocker.MagicMock(), host="rhasspy.local", - port=1883, - tls=False, + port=8883, username="rhasspy-hermes-app-test", password="covfefe", ) assert app.args.host == "rhasspy.home" - assert app.args.port == 8883 - assert app.args.tls == True + assert app.args.port == 1883 assert app.args.username == "rhasspy-hermes-app" assert app.args.password == "test" From 4a2c6346f0dcf30676e2a8a05150fb66c84c2d67 Mon Sep 17 00:00:00 2001 From: Max Bachmann Date: Fri, 28 Aug 2020 13:07:20 +0200 Subject: [PATCH 10/12] fix argument precedence --- rhasspyhermes_app/__init__.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/rhasspyhermes_app/__init__.py b/rhasspyhermes_app/__init__.py index 6c2deb8..a1c91b9 100644 --- a/rhasspyhermes_app/__init__.py +++ b/rhasspyhermes_app/__init__.py @@ -3,6 +3,7 @@ import asyncio import logging import re +from copy import deepcopy from dataclasses import dataclass from typing import Awaitable, Callable, Dict, List, Optional, Union @@ -104,18 +105,18 @@ def __init__( # Add default arguments hermes_cli.add_hermes_args(parser) - # Parse command-line arguments + # overwrite argument defaults inside parser with argparse.SUPPRESS + # so arguments that are not provided get ignored + suppress_parser = deepcopy(parser) + for action in vars(suppress_parser)["_actions"]: + action.default = argparse.SUPPRESS + + supplied_args = vars(suppress_parser.parse_args()) + default_args = vars(parser.parse_args([])) + # Command-line arguments take precedence over the arguments of the HermesApp.__init__ - args_dict = vars(parser.parse_args()) - default_args_dict = vars(parser.parse_args([])) - # Remove the arguments which have their default values - non_default_args_dict = dict( - set(args_dict.items()) - set(default_args_dict.items()) - ) - # Let the non-default arguments take precedence over the object arguments - args = {**kwargs, **non_default_args_dict} - # Merge these back with the original arguments, taking into account precedence - self.args = argparse.Namespace(**{**args_dict, **args}) + args = {**default_args, **kwargs, **supplied_args} + self.args = argparse.Namespace(**args) # Set up logging hermes_cli.setup_logging(self.args) From 882276ed8d2159aef612599dd9b24f2c08b3b1ce Mon Sep 17 00:00:00 2001 From: Max Bachmann Date: Fri, 28 Aug 2020 13:09:54 +0200 Subject: [PATCH 11/12] do not convert to dict first --- rhasspyhermes_app/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rhasspyhermes_app/__init__.py b/rhasspyhermes_app/__init__.py index a1c91b9..d6386b9 100644 --- a/rhasspyhermes_app/__init__.py +++ b/rhasspyhermes_app/__init__.py @@ -108,7 +108,7 @@ def __init__( # overwrite argument defaults inside parser with argparse.SUPPRESS # so arguments that are not provided get ignored suppress_parser = deepcopy(parser) - for action in vars(suppress_parser)["_actions"]: + for action in suppress_parser._actions: action.default = argparse.SUPPRESS supplied_args = vars(suppress_parser.parse_args()) From 3cb706f3cc25858eda2f522b6fe9cc91b2b45348 Mon Sep 17 00:00:00 2001 From: Koen Vervloesem Date: Fri, 28 Aug 2020 13:39:36 +0200 Subject: [PATCH 12/12] Add test for supplying your own argument parser --- tests/test_arguments.py | 45 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/tests/test_arguments.py b/tests/test_arguments.py index da7a0f7..4f47220 100644 --- a/tests/test_arguments.py +++ b/tests/test_arguments.py @@ -75,7 +75,7 @@ def test_if_cli_arguments_overwrite_init_arguments(mocker): "--username", "rhasspy-hermes-app", "--password", - "test" + "test", ], ) app = HermesApp( @@ -91,3 +91,46 @@ def test_if_cli_arguments_overwrite_init_arguments(mocker): assert app.args.port == 1883 assert app.args.username == "rhasspy-hermes-app" assert app.args.password == "test" + + +def test_if_cli_arguments_overwrite_init_arguments_with_argument_parser(mocker): + """Test whether arguments from the command line overwrite arguments to a HermesApp object + if the user supplies their own ArgumentParser object.""" + mocker.patch( + "sys.argv", + [ + "rhasspy-hermes-app-test", + "--host", + "rhasspy.home", + "--port", + "1883", + "--username", + "rhasspy-hermes-app", + "--password", + "test", + "--test-argument", + "foobar", + "--test-flag", + ], + ) + parser = argparse.ArgumentParser(prog="rhasspy-hermes-app-test") + parser.add_argument("--test-argument", default="foo") + parser.add_argument("--test-flag", action="store_true") + + app = HermesApp( + "Test arguments in init", + parser=parser, + mqtt_client=mocker.MagicMock(), + host="rhasspy.local", + port=8883, + username="rhasspy-hermes-app-test", + password="covfefe", + test_argument="bar", + ) + + assert app.args.host == "rhasspy.home" + assert app.args.port == 1883 + assert app.args.username == "rhasspy-hermes-app" + assert app.args.password == "test" + assert app.args.test_argument == "foobar" + assert app.args.test_flag == True