From 84d4a89b7d21f7f3cf0a0836cee14bbcec2d6def Mon Sep 17 00:00:00 2001 From: Philippe Cote-Boucher Date: Thu, 23 Jul 2020 14:21:13 -0400 Subject: [PATCH 01/13] show error messages in logs even if they're sent as http response --- rasa/server.py | 1 + 1 file changed, 1 insertion(+) diff --git a/rasa/server.py b/rasa/server.py index 7aa1c9e3affa..bfe6aa6b8a49 100644 --- a/rasa/server.py +++ b/rasa/server.py @@ -76,6 +76,7 @@ def __init__( "code": status, } self.status = status + logger.error(message) def _docs(sub_url: Text) -> Text: From 5faa4bb157f0c4e69ba7844425fdc5d9c36d6661 Mon Sep 17 00:00:00 2001 From: Philippe Cote-Boucher Date: Thu, 23 Jul 2020 14:21:56 -0400 Subject: [PATCH 02/13] allow rasa to run without interruption when model is outdated --- rasa/core/interpreter.py | 7 ++++++- rasa/core/policies/ensemble.py | 8 ++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/rasa/core/interpreter.py b/rasa/core/interpreter.py index 3b889708f875..5c73cc888b8d 100644 --- a/rasa/core/interpreter.py +++ b/rasa/core/interpreter.py @@ -37,7 +37,12 @@ def create( if isinstance(obj, NaturalLanguageInterpreter): return obj elif isinstance(obj, str) and os.path.exists(obj): - return RasaNLUInterpreter(model_directory=obj) + from rasa.nlu.model import UnsupportedModelError + try: + return RasaNLUInterpreter(model_directory=obj) + except UnsupportedModelError as e: + logger.warning(e.message) + return RegexInterpreter() elif isinstance(obj, str) and not os.path.exists(obj): # user passed in a string, but file does not exist logger.warning( diff --git a/rasa/core/policies/ensemble.py b/rasa/core/policies/ensemble.py index a16a12482af9..f2097cfb1b4d 100644 --- a/rasa/core/policies/ensemble.py +++ b/rasa/core/policies/ensemble.py @@ -318,11 +318,15 @@ def _ensure_loaded_policy(cls, policy, policy_cls, policy_name: Text): ) @classmethod - def load(cls, path: Text) -> "PolicyEnsemble": + def load(cls, path: Text) -> Optional["PolicyEnsemble"]: """Loads policy and domain specification from storage""" metadata = cls.load_metadata(path) - cls.ensure_model_compatibility(metadata) + try: + cls.ensure_model_compatibility(metadata) + except UnsupportedDialogueModelError as e: + logger.warning(e.message) + return None policies = [] for i, policy_name in enumerate(metadata["policy_names"]): policy_cls = registry.policy_from_module_path(policy_name) From b2e9b63befb6d2efb97740a11432f82454207ea0 Mon Sep 17 00:00:00 2001 From: Philippe Cote-Boucher Date: Fri, 24 Jul 2020 11:36:45 -0400 Subject: [PATCH 03/13] changelog entry --- changelog/6276.improvement.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/6276.improvement.rst diff --git a/changelog/6276.improvement.rst b/changelog/6276.improvement.rst new file mode 100644 index 000000000000..0b97da59dee8 --- /dev/null +++ b/changelog/6276.improvement.rst @@ -0,0 +1 @@ +Allow Rasa to boot even when UnsupportedDialogueModelError or UnsupportedModelError occurs. Forward HTTP Error responses to standard log output. From 8445ff997c3870039359730994064b8278bd38ab Mon Sep 17 00:00:00 2001 From: Philippe Cote-Boucher Date: Fri, 24 Jul 2020 11:45:01 -0400 Subject: [PATCH 04/13] black formatting --- rasa/core/interpreter.py | 1 + 1 file changed, 1 insertion(+) diff --git a/rasa/core/interpreter.py b/rasa/core/interpreter.py index 5c73cc888b8d..0499223c8127 100644 --- a/rasa/core/interpreter.py +++ b/rasa/core/interpreter.py @@ -38,6 +38,7 @@ def create( return obj elif isinstance(obj, str) and os.path.exists(obj): from rasa.nlu.model import UnsupportedModelError + try: return RasaNLUInterpreter(model_directory=obj) except UnsupportedModelError as e: From 8f3619aad3113c977facda504c87fad71b594685 Mon Sep 17 00:00:00 2001 From: Philippe Cote-Boucher Date: Thu, 23 Jul 2020 14:04:30 -0400 Subject: [PATCH 05/13] use NaturalLanguageInterpreter factory in agent internal method instead of creating class directly --- rasa/core/agent.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/rasa/core/agent.py b/rasa/core/agent.py index f1772db7e18b..e608b0f7db74 100644 --- a/rasa/core/agent.py +++ b/rasa/core/agent.py @@ -85,9 +85,7 @@ def _load_interpreter( The NLU interpreter. """ if nlu_path: - from rasa.core.interpreter import RasaNLUInterpreter - - return RasaNLUInterpreter(model_directory=nlu_path) + return NaturalLanguageInterpreter.create(nlu_path) return agent.interpreter or RegexInterpreter() From 6ac2eb0315e0ddfa2cee35c0d3dea25f779e48b3 Mon Sep 17 00:00:00 2001 From: Philippe Cote-Boucher Date: Wed, 12 Aug 2020 09:41:08 -0400 Subject: [PATCH 06/13] move model loading error handling to load_agent_on_start --- rasa/core/interpreter.py | 8 +------- rasa/core/policies/ensemble.py | 8 ++------ rasa/core/run.py | 26 ++++++++++++++++---------- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/rasa/core/interpreter.py b/rasa/core/interpreter.py index 0499223c8127..3b889708f875 100644 --- a/rasa/core/interpreter.py +++ b/rasa/core/interpreter.py @@ -37,13 +37,7 @@ def create( if isinstance(obj, NaturalLanguageInterpreter): return obj elif isinstance(obj, str) and os.path.exists(obj): - from rasa.nlu.model import UnsupportedModelError - - try: - return RasaNLUInterpreter(model_directory=obj) - except UnsupportedModelError as e: - logger.warning(e.message) - return RegexInterpreter() + return RasaNLUInterpreter(model_directory=obj) elif isinstance(obj, str) and not os.path.exists(obj): # user passed in a string, but file does not exist logger.warning( diff --git a/rasa/core/policies/ensemble.py b/rasa/core/policies/ensemble.py index f2097cfb1b4d..a16a12482af9 100644 --- a/rasa/core/policies/ensemble.py +++ b/rasa/core/policies/ensemble.py @@ -318,15 +318,11 @@ def _ensure_loaded_policy(cls, policy, policy_cls, policy_name: Text): ) @classmethod - def load(cls, path: Text) -> Optional["PolicyEnsemble"]: + def load(cls, path: Text) -> "PolicyEnsemble": """Loads policy and domain specification from storage""" metadata = cls.load_metadata(path) - try: - cls.ensure_model_compatibility(metadata) - except UnsupportedDialogueModelError as e: - logger.warning(e.message) - return None + cls.ensure_model_compatibility(metadata) policies = [] for i, policy_name in enumerate(metadata["policy_names"]): policy_cls = registry.policy_from_module_path(policy_name) diff --git a/rasa/core/run.py b/rasa/core/run.py index a06f6d9930dc..8bdc2b668a43 100644 --- a/rasa/core/run.py +++ b/rasa/core/run.py @@ -246,16 +246,22 @@ async def load_agent_on_start( model_server = endpoints.model if endpoints and endpoints.model else None - app.agent = await agent.load_agent( - model_path, - model_server=model_server, - remote_storage=remote_storage, - interpreter=_interpreter, - generator=endpoints.nlg, - tracker_store=_tracker_store, - lock_store=_lock_store, - action_endpoint=endpoints.action, - ) + try: + app.agent = await agent.load_agent( + model_path, + model_server=model_server, + remote_storage=remote_storage, + interpreter=_interpreter, + generator=endpoints.nlg, + tracker_store=_tracker_store, + lock_store=_lock_store, + action_endpoint=endpoints.action, + ) + except Exception as e: + raise_warning( + f"The model at '{model_path}' could not be loaded. " f"Error: {e}" + ) + app.agent = None if not app.agent: raise_warning( From fadee4da939137294a19665766c8ccd4f51a3cfb Mon Sep 17 00:00:00 2001 From: Philippe Cote-Boucher Date: Wed, 12 Aug 2020 09:42:41 -0400 Subject: [PATCH 07/13] update changelog --- changelog/6276.improvement.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/6276.improvement.rst b/changelog/6276.improvement.rst index 0b97da59dee8..5955add6d750 100644 --- a/changelog/6276.improvement.rst +++ b/changelog/6276.improvement.rst @@ -1 +1 @@ -Allow Rasa to boot even when UnsupportedDialogueModelError or UnsupportedModelError occurs. Forward HTTP Error responses to standard log output. +Allow Rasa to boot when model loading exception occurs. Forward HTTP Error responses to standard log output. From bc4370ce02fd323233c7e5f7c6f25f9eb140f94b Mon Sep 17 00:00:00 2001 From: Philippe Cote-Boucher Date: Mon, 17 Aug 2020 18:12:27 -0400 Subject: [PATCH 08/13] add unit tests for load_agent_on_start --- tests/core/test_run.py | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/tests/core/test_run.py b/tests/core/test_run.py index f6028e26947e..5ef6bde6c3e8 100644 --- a/tests/core/test_run.py +++ b/tests/core/test_run.py @@ -1,4 +1,6 @@ -from rasa.core import run +import pytest +from rasa.core import run, interpreter, policies, domain +from rasa.core.utils import AvailableEndpoints CREDENTIALS_FILE = "examples/moodbot/credentials.yml" @@ -40,3 +42,38 @@ def test_create_single_input_channels_by_class_wo_credentials(): assert len(channels) == 1 assert channels[0].name() == "rest" + + +async def test_load_agent_on_start_with_good_model_file( + trained_rasa_model, rasa_server, loop, +): + agent = await run.load_agent_on_start( + trained_rasa_model, AvailableEndpoints(), None, rasa_server, loop, + ) + + assert isinstance(agent.interpreter, interpreter.RasaNLUInterpreter) + assert isinstance(agent.policy_ensemble, policies.PolicyEnsemble) + assert isinstance(agent.domain, domain.Domain) + + +async def test_load_agent_on_start_with_bad_model_file( + tmpdir, rasa_server, loop, +): + from pathlib import Path + + fake_model = Path(tmpdir) / "fake_model.tar.gz" + fake_model.touch() + fake_model_path = str(fake_model) + + with pytest.warns(UserWarning) as warnings: + agent = await run.load_agent_on_start( + fake_model_path, AvailableEndpoints(), None, rasa_server, loop, + ) + assert any( + "fake_model.tar.gz' could not be loaded" in str(w.message) for w in warnings + ) + + # Fallback agent was loaded even if model was unusable + assert isinstance(agent.interpreter, interpreter.RegexInterpreter) + assert agent.policy_ensemble is None + assert isinstance(agent.domain, domain.Domain) From 2f324e0811c863aa72adff562fd802e3127752ac Mon Sep 17 00:00:00 2001 From: Philippe Cote-Boucher Date: Mon, 17 Aug 2020 18:22:30 -0400 Subject: [PATCH 09/13] remove whitespace on blank line --- tests/core/test_run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/test_run.py b/tests/core/test_run.py index 5acfb4986176..b2b2111d6f9d 100644 --- a/tests/core/test_run.py +++ b/tests/core/test_run.py @@ -72,7 +72,7 @@ async def test_load_agent_on_start_with_bad_model_file( assert any( "fake_model.tar.gz' could not be loaded" in str(w.message) for w in warnings ) - + # Fallback agent was loaded even if model was unusable assert isinstance(agent.interpreter, interpreter.RegexInterpreter) assert agent.policy_ensemble is None From 1cbcbd36652dba89ed49cefb4b3b73d289ca4b1f Mon Sep 17 00:00:00 2001 From: Philippe Cote-Boucher Date: Tue, 18 Aug 2020 08:39:46 -0400 Subject: [PATCH 10/13] md changelog entry --- changelog/{6276.improvement.rst => 6276.improvement.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/{6276.improvement.rst => 6276.improvement.md} (100%) diff --git a/changelog/6276.improvement.rst b/changelog/6276.improvement.md similarity index 100% rename from changelog/6276.improvement.rst rename to changelog/6276.improvement.md From 9296f2139df435b15ae4a586307a165f70d3ce1b Mon Sep 17 00:00:00 2001 From: Philippe Cote-Boucher Date: Tue, 18 Aug 2020 08:40:16 -0400 Subject: [PATCH 11/13] remove unused loop argument --- rasa/core/run.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rasa/core/run.py b/rasa/core/run.py index 8bdc2b668a43..384beb8f6f7e 100644 --- a/rasa/core/run.py +++ b/rasa/core/run.py @@ -224,12 +224,11 @@ async def load_agent_on_start( endpoints: AvailableEndpoints, remote_storage: Optional[Text], app: Sanic, - loop: Text, ): """Load an agent. Used to be scheduled on server start - (hence the `app` and `loop` arguments).""" + (hence the `app` argument).""" # noinspection PyBroadException try: From e8c348b90cd096bf163ebddd0ff9e9b5c212d629 Mon Sep 17 00:00:00 2001 From: Philippe Cote-Boucher Date: Tue, 18 Aug 2020 08:41:26 -0400 Subject: [PATCH 12/13] add missing type annotations --- tests/core/test_run.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/core/test_run.py b/tests/core/test_run.py index b2b2111d6f9d..19b7f0f07114 100644 --- a/tests/core/test_run.py +++ b/tests/core/test_run.py @@ -1,4 +1,7 @@ import pytest +from typing import Text +from sanic import Sanic +from pathlib import Path from rasa.core import run, interpreter, policies, domain from rasa.core.utils import AvailableEndpoints @@ -45,10 +48,10 @@ def test_create_single_input_channels_by_class_wo_credentials(): async def test_load_agent_on_start_with_good_model_file( - trained_rasa_model, rasa_server, loop, + trained_rasa_model: Text, rasa_server: Sanic, ): agent = await run.load_agent_on_start( - trained_rasa_model, AvailableEndpoints(), None, rasa_server, loop, + trained_rasa_model, AvailableEndpoints(), None, rasa_server, ) assert isinstance(agent.interpreter, interpreter.RasaNLUInterpreter) @@ -57,17 +60,15 @@ async def test_load_agent_on_start_with_good_model_file( async def test_load_agent_on_start_with_bad_model_file( - tmpdir, rasa_server, loop, + tmp_path: Path, rasa_server: Sanic, ): - from pathlib import Path - - fake_model = Path(tmpdir) / "fake_model.tar.gz" + fake_model = tmp_path / "fake_model.tar.gz" fake_model.touch() fake_model_path = str(fake_model) with pytest.warns(UserWarning) as warnings: agent = await run.load_agent_on_start( - fake_model_path, AvailableEndpoints(), None, rasa_server, loop, + fake_model_path, AvailableEndpoints(), None, rasa_server, ) assert any( "fake_model.tar.gz' could not be loaded" in str(w.message) for w in warnings From a4adb0beceb3e876aa6007a3017f3f4e831041cd Mon Sep 17 00:00:00 2001 From: Philippe Cote-Boucher Date: Thu, 20 Aug 2020 09:10:19 -0400 Subject: [PATCH 13/13] readd loop argument to load_agent_on_start listener --- rasa/core/run.py | 4 +++- tests/core/test_run.py | 9 +++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/rasa/core/run.py b/rasa/core/run.py index 384beb8f6f7e..c7f13a1cd4c7 100644 --- a/rasa/core/run.py +++ b/rasa/core/run.py @@ -23,6 +23,7 @@ from rasa.core.utils import AvailableEndpoints from rasa.utils.common import raise_warning from sanic import Sanic +from asyncio import AbstractEventLoop logger = logging.getLogger() # get the root logger @@ -224,11 +225,12 @@ async def load_agent_on_start( endpoints: AvailableEndpoints, remote_storage: Optional[Text], app: Sanic, + loop: AbstractEventLoop, ): """Load an agent. Used to be scheduled on server start - (hence the `app` argument).""" + (hence the `app` and `loop` arguments).""" # noinspection PyBroadException try: diff --git a/tests/core/test_run.py b/tests/core/test_run.py index 19b7f0f07114..de8df1c85d7b 100644 --- a/tests/core/test_run.py +++ b/tests/core/test_run.py @@ -1,6 +1,7 @@ import pytest from typing import Text from sanic import Sanic +from asyncio import AbstractEventLoop from pathlib import Path from rasa.core import run, interpreter, policies, domain from rasa.core.utils import AvailableEndpoints @@ -48,10 +49,10 @@ def test_create_single_input_channels_by_class_wo_credentials(): async def test_load_agent_on_start_with_good_model_file( - trained_rasa_model: Text, rasa_server: Sanic, + trained_rasa_model: Text, rasa_server: Sanic, loop: AbstractEventLoop, ): agent = await run.load_agent_on_start( - trained_rasa_model, AvailableEndpoints(), None, rasa_server, + trained_rasa_model, AvailableEndpoints(), None, rasa_server, loop, ) assert isinstance(agent.interpreter, interpreter.RasaNLUInterpreter) @@ -60,7 +61,7 @@ async def test_load_agent_on_start_with_good_model_file( async def test_load_agent_on_start_with_bad_model_file( - tmp_path: Path, rasa_server: Sanic, + tmp_path: Path, rasa_server: Sanic, loop: AbstractEventLoop, ): fake_model = tmp_path / "fake_model.tar.gz" fake_model.touch() @@ -68,7 +69,7 @@ async def test_load_agent_on_start_with_bad_model_file( with pytest.warns(UserWarning) as warnings: agent = await run.load_agent_on_start( - fake_model_path, AvailableEndpoints(), None, rasa_server, + fake_model_path, AvailableEndpoints(), None, rasa_server, loop, ) assert any( "fake_model.tar.gz' could not be loaded" in str(w.message) for w in warnings