From ff61e8f7e7d6447048a5f66a33f7e98ae85f7930 Mon Sep 17 00:00:00 2001 From: RanaMostafaAbdElMohsen Date: Tue, 2 Jul 2019 23:51:05 +0200 Subject: [PATCH 01/10] fixed crash when --domain not provided for rasa train command An Error message will be displayed in case of invalid domain file provided or no domain file provided at all for 'rasa train core' only --- rasa/train.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/rasa/train.py b/rasa/train.py index d4fcc61acb49..eab1010adf6b 100644 --- a/rasa/train.py +++ b/rasa/train.py @@ -247,6 +247,13 @@ async def train_core_async( skill_imports = SkillSelector.load(config, stories) + if isinstance(domain, type(None)): + print_error( + "Core training is skipped because no domain was found. " + "Please specify a valid domain using '--domain' argument or check if provided domain file does exists" + ) + return None + if isinstance(domain, str): try: domain = Domain.load(domain, skill_imports) From 3f7b4a1a20b9b4e88ec895211095a00ad7533ca6 Mon Sep 17 00:00:00 2001 From: RanaMostafaAbdElMohsen Date: Wed, 3 Jul 2019 23:58:12 +0200 Subject: [PATCH 02/10] Fix train command and added new test cases --- rasa/train.py | 37 +++++++++++----------- tests/cli/test_rasa_train.py | 61 ++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 19 deletions(-) diff --git a/rasa/train.py b/rasa/train.py index eab1010adf6b..52f5ca8e3bc6 100644 --- a/rasa/train.py +++ b/rasa/train.py @@ -68,19 +68,26 @@ async def train_async( train_path = tempfile.mkdtemp() skill_imports = SkillSelector.load(config, training_files) + + story_directory, nlu_data_directory = data.get_core_nlu_directories( + training_files, skill_imports + ) + try: domain = Domain.load(domain, skill_imports) domain.check_missing_templates() except InvalidDomain as e: - print_error( - "Could not load domain due to error: {} \nTo specify a valid domain " - "path, use the '--domain' argument.".format(e) + print_warning( + "Core training is skipped because no domain was found. " + "Please specify a valid domain using '--domain' argument or check if provided domain file does exists" + ) + return _train_nlu_with_validated_data( + config=config, + nlu_data_directory=nlu_data_directory, + output=output_path, + fixed_model_name=fixed_model_name, ) - return None - story_directory, nlu_data_directory = data.get_core_nlu_directories( - training_files, skill_imports - ) new_fingerprint = model.model_fingerprint( config, domain, nlu_data_directory, story_directory ) @@ -247,24 +254,16 @@ async def train_core_async( skill_imports = SkillSelector.load(config, stories) - if isinstance(domain, type(None)): + try: + domain = Domain.load(domain, skill_imports) + domain.check_missing_templates() + except InvalidDomain: print_error( "Core training is skipped because no domain was found. " "Please specify a valid domain using '--domain' argument or check if provided domain file does exists" ) return None - if isinstance(domain, str): - try: - domain = Domain.load(domain, skill_imports) - domain.check_missing_templates() - except InvalidDomain as e: - print_error( - "Could not load domain due to: '{}'. To specify a valid domain path " - "use the '--domain' argument.".format(e) - ) - return None - story_directory = data.get_core_directory(stories, skill_imports) if not os.listdir(story_directory): diff --git a/tests/cli/test_rasa_train.py b/tests/cli/test_rasa_train.py index 744a1a2d4652..57df47cde7c3 100644 --- a/tests/cli/test_rasa_train.py +++ b/tests/cli/test_rasa_train.py @@ -37,6 +37,49 @@ def test_train(run_in_default_project): assert os.path.basename(files[0]) == "test-model.tar.gz" +def test_train(run_in_default_project): + temp_dir = os.getcwd() + + run_in_default_project( + "train", + "-c", + "config.yml", + "-d", + "domain.yml", + "--data", + "data", + "--out", + "train_models", + "--fixed-model-name", + "test-model", + ) + + assert os.path.exists(os.path.join(temp_dir, "train_models")) + files = list_files(os.path.join(temp_dir, "train_models")) + assert len(files) == 1 + assert os.path.basename(files[0]) == "test-model.tar.gz" + + +def test_train_no_domain_exists(run_in_default_project): + temp_dir = os.getcwd() + + run_in_default_project( + "train", + "-c", + "config.yml", + "--data", + "data", + "--out", + "train_models", + "--fixed-model-name", + "test-model", + ) + assert os.path.exists("train_models") + files = list_files("train_models") + assert len(files) == 1 + assert os.path.basename(files[0]).startswith("nlu-") + + def test_train_skip_on_model_not_changed(run_in_default_project): temp_dir = os.getcwd() @@ -118,6 +161,24 @@ def test_train_core(run_in_default_project): assert os.path.isfile("train_rasa_models/rasa-model.tar.gz") +def test_train_core_no_domain_exists(run_in_default_project): + run_in_default_project( + "train", + "core", + "-c", + "config.yml", + "--stories", + "data", + "--out", + "train_rasa_models", + "--fixed-model-name", + "rasa-model", + ) + + assert not os.path.exists("train_rasa_models/rasa-model.tar.gz") + assert not os.path.isfile("train_rasa_models/rasa-model.tar.gz") + + def test_train_nlu(run_in_default_project): run_in_default_project( "train", From fdfeb3826a440f80c6162188227e56394d9a2d23 Mon Sep 17 00:00:00 2001 From: RanaMostafaAbdElMohsen Date: Sat, 6 Jul 2019 20:16:02 +0200 Subject: [PATCH 03/10] Changes in testcases --- tests/cli/test_rasa_train.py | 45 +++++++++++------------------------- 1 file changed, 13 insertions(+), 32 deletions(-) diff --git a/tests/cli/test_rasa_train.py b/tests/cli/test_rasa_train.py index 57df47cde7c3..a0309fb9fe74 100644 --- a/tests/cli/test_rasa_train.py +++ b/tests/cli/test_rasa_train.py @@ -14,29 +14,6 @@ from rasa.nlu.utils import list_files -def test_train(run_in_default_project): - temp_dir = os.getcwd() - - run_in_default_project( - "train", - "-c", - "config.yml", - "-d", - "domain.yml", - "--data", - "data", - "--out", - "train_models", - "--fixed-model-name", - "test-model", - ) - - assert os.path.exists(os.path.join(temp_dir, "train_models")) - files = list_files(os.path.join(temp_dir, "train_models")) - assert len(files) == 1 - assert os.path.basename(files[0]) == "test-model.tar.gz" - - def test_train(run_in_default_project): temp_dir = os.getcwd() @@ -70,14 +47,14 @@ def test_train_no_domain_exists(run_in_default_project): "--data", "data", "--out", - "train_models", + "train_models_no_domain", "--fixed-model-name", - "test-model", + "nlu-model-only", ) - assert os.path.exists("train_models") - files = list_files("train_models") + assert os.path.exists("train_models_no_domain") + files = list_files("train_models_no_domain") assert len(files) == 1 - assert os.path.basename(files[0]).startswith("nlu-") + def test_train_skip_on_model_not_changed(run_in_default_project): @@ -162,21 +139,25 @@ def test_train_core(run_in_default_project): def test_train_core_no_domain_exists(run_in_default_project): + + os.remove("domain.yml"); run_in_default_project( "train", "core", - "-c", + "--config", "config.yml", + "--domain", + "domain.yml", "--stories", "data", "--out", - "train_rasa_models", + "train_rasa_models_no_domain", "--fixed-model-name", "rasa-model", ) - assert not os.path.exists("train_rasa_models/rasa-model.tar.gz") - assert not os.path.isfile("train_rasa_models/rasa-model.tar.gz") + assert not os.path.exists("train_rasa_models_no_domain/rasa-model.tar.gz") + assert not os.path.isfile("train_rasa_models_no_domain/rasa-model.tar.gz") def test_train_nlu(run_in_default_project): From c0e2c0fba6a09c6db141a9959951dfd988e09070 Mon Sep 17 00:00:00 2001 From: RanaMostafaAbdElMohsen Date: Sat, 6 Jul 2019 21:01:46 +0200 Subject: [PATCH 04/10] Modified testcases to be successful - Additional changes for nlu/metadata.json to make sure nlu has been trained --- tests/cli/test_rasa_train.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/cli/test_rasa_train.py b/tests/cli/test_rasa_train.py index a0309fb9fe74..d46b577ef974 100644 --- a/tests/cli/test_rasa_train.py +++ b/tests/cli/test_rasa_train.py @@ -4,6 +4,8 @@ import pytest +from rasa import model + from rasa.cli.train import _get_valid_config from rasa.constants import ( CONFIG_MANDATORY_KEYS_CORE, @@ -38,8 +40,8 @@ def test_train(run_in_default_project): def test_train_no_domain_exists(run_in_default_project): - temp_dir = os.getcwd() - + + os.remove("domain.yml") run_in_default_project( "train", "-c", @@ -51,9 +53,18 @@ def test_train_no_domain_exists(run_in_default_project): "--fixed-model-name", "nlu-model-only", ) + assert os.path.exists("train_models_no_domain") files = list_files("train_models_no_domain") assert len(files) == 1 + + trained_model_path = "train_models_no_domain/nlu-model-only.tar.gz" + unpacked = model.unpack_model(trained_model_path) + + metadata_path = os.path.join(unpacked, "nlu", "metadata.json") + assert os.path.exists(metadata_path) + + @@ -140,14 +151,14 @@ def test_train_core(run_in_default_project): def test_train_core_no_domain_exists(run_in_default_project): - os.remove("domain.yml"); + os.remove("domain.yml"); run_in_default_project( "train", "core", "--config", "config.yml", "--domain", - "domain.yml", + "domain1.yml", "--stories", "data", "--out", From f12adbf6aa2618a7f9e5cf172f0f314cf270a525 Mon Sep 17 00:00:00 2001 From: RanaMostafaAbdElMohsen Date: Sun, 7 Jul 2019 23:48:38 +0200 Subject: [PATCH 05/10] Fix Code Formatting --- rasa/train.py | 22 ++++++++++++++-------- tests/cli/test_rasa_train.py | 10 ++++------ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/rasa/train.py b/rasa/train.py index 534c7e807a14..48d4f71beffb 100644 --- a/rasa/train.py +++ b/rasa/train.py @@ -72,8 +72,8 @@ async def train_async( domain = Domain.load(domain, skill_imports) domain.check_missing_templates() except InvalidDomain as e: - domain=None - + domain = None + story_directory, nlu_data_directory = data.get_core_nlu_directories( training_files, skill_imports ) @@ -84,7 +84,9 @@ async def train_async( story = stack.enter_context(TempDirectoryPath(story_directory)) if domain is None: - return handle_domain_if_not_exists(config,nlu_data_directory,output_path,fixed_model_name) + return handle_domain_if_not_exists( + config, nlu_data_directory, output_path, fixed_model_name + ) return await _train_async_internal( domain, @@ -97,15 +99,19 @@ async def train_async( fixed_model_name, kwargs, ) - + if domain is None: - return handle_domain_if_not_exists(config,nlu_data_directory,output_path,fixed_model_name) + return handle_domain_if_not_exists( + config, nlu_data_directory, output_path, fixed_model_name + ) -def handle_domain_if_not_exists(config,nlu_data_directory,output_path,fixed_model_name): +def handle_domain_if_not_exists( + config, nlu_data_directory, output_path, fixed_model_name +): print_warning( - "Core training is skipped because no domain was found. " - "Please specify a valid domain using '--domain' argument or check if provided domain file does exists" + "Core training is skipped because no domain was found. " + "Please specify a valid domain using '--domain' argument or check if provided domain file does exists" ) return _train_nlu_with_validated_data( config=config, diff --git a/tests/cli/test_rasa_train.py b/tests/cli/test_rasa_train.py index 2f39dc9378b9..e483095b82bc 100644 --- a/tests/cli/test_rasa_train.py +++ b/tests/cli/test_rasa_train.py @@ -40,7 +40,7 @@ def test_train(run_in_default_project): def test_train_no_domain_exists(run_in_default_project): - + os.remove("domain.yml") run_in_default_project( "train", @@ -64,9 +64,6 @@ def test_train_no_domain_exists(run_in_default_project): metadata_path = os.path.join(unpacked, "nlu", "metadata.json") assert os.path.exists(metadata_path) - - - def test_train_skip_on_model_not_changed(run_in_default_project): temp_dir = os.getcwd() @@ -151,7 +148,7 @@ def test_train_core(run_in_default_project): def test_train_core_no_domain_exists(run_in_default_project): - os.remove("domain.yml"); + os.remove("domain.yml") run_in_default_project( "train", "core", @@ -169,7 +166,8 @@ def test_train_core_no_domain_exists(run_in_default_project): assert not os.path.exists("train_rasa_models_no_domain/rasa-model.tar.gz") assert not os.path.isfile("train_rasa_models_no_domain/rasa-model.tar.gz") - + + def count_rasa_temp_files(): count = 0 for entry in os.scandir(tempfile.gettempdir()): From 4348f22460dbe5261fb6854716ee7d4f18911c19 Mon Sep 17 00:00:00 2001 From: RanaMostafaAbdElMohsen Date: Mon, 8 Jul 2019 00:09:23 +0200 Subject: [PATCH 06/10] Fix linting error --- rasa/train.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rasa/train.py b/rasa/train.py index 48d4f71beffb..112d5fe9916f 100644 --- a/rasa/train.py +++ b/rasa/train.py @@ -71,7 +71,7 @@ async def train_async( try: domain = Domain.load(domain, skill_imports) domain.check_missing_templates() - except InvalidDomain as e: + except InvalidDomain: domain = None story_directory, nlu_data_directory = data.get_core_nlu_directories( From afa566f2fb37962c8b7cf084d4a1b2620bab20b1 Mon Sep 17 00:00:00 2001 From: RanaMostafa Date: Mon, 8 Jul 2019 18:44:45 +0200 Subject: [PATCH 07/10] Small ammendations in print_warning if domain.yml is missing --- rasa/train.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/rasa/train.py b/rasa/train.py index 112d5fe9916f..b2906b7fe8d8 100644 --- a/rasa/train.py +++ b/rasa/train.py @@ -109,16 +109,17 @@ async def train_async( def handle_domain_if_not_exists( config, nlu_data_directory, output_path, fixed_model_name ): - print_warning( - "Core training is skipped because no domain was found. " - "Please specify a valid domain using '--domain' argument or check if provided domain file does exists" - ) - return _train_nlu_with_validated_data( + nlu_model_only = _train_nlu_with_validated_data( config=config, nlu_data_directory=nlu_data_directory, output=output_path, fixed_model_name=fixed_model_name, ) + print_warning( + "Core training is skipped because no domain was found. " + "Please specify a valid domain using '--domain' argument or check if the provided domain file exists." + ) + return nlu_model_only async def _train_async_internal( @@ -320,7 +321,7 @@ async def train_core_async( except InvalidDomain: print_error( "Core training is skipped because no domain was found. " - "Please specify a valid domain using '--domain' argument or check if provided domain file does exists" + "Please specify a valid domain using '--domain' argument or check if the provided domain file exists." ) return None From 2cbb5b1ce86b75a571f5a9033e2b8da7c617aa7a Mon Sep 17 00:00:00 2001 From: RanaMostafa Date: Mon, 8 Jul 2019 18:48:06 +0200 Subject: [PATCH 08/10] Added in print_warning msg that only nlu_model was created --- rasa/train.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rasa/train.py b/rasa/train.py index b2906b7fe8d8..0802536fba4e 100644 --- a/rasa/train.py +++ b/rasa/train.py @@ -116,7 +116,7 @@ def handle_domain_if_not_exists( fixed_model_name=fixed_model_name, ) print_warning( - "Core training is skipped because no domain was found. " + "Core training was skipped because no valid domain file was found. Only an nlu-model was created." "Please specify a valid domain using '--domain' argument or check if the provided domain file exists." ) return nlu_model_only @@ -320,7 +320,7 @@ async def train_core_async( domain.check_missing_templates() except InvalidDomain: print_error( - "Core training is skipped because no domain was found. " + "Core training was skipped because no valid domain file was found. " "Please specify a valid domain using '--domain' argument or check if the provided domain file exists." ) return None From 460708917be539370f4aa166fc8bca61104dc2fc Mon Sep 17 00:00:00 2001 From: RanaMostafa Date: Mon, 8 Jul 2019 21:08:49 +0200 Subject: [PATCH 09/10] Updated CHANGELOG.rst for rasa_train_command_fix to go under fixed section --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 790785a1f481..12fe22871bee 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -56,7 +56,7 @@ Fixed - fixed bug where facebook quick replies were not rendering - take FB quick reply payload rather than text as input - fixed bug where `training_data` path in `metadata.json` was an absolute path - +- ``rasa train core`` no longer crashes without a ``--domain`` arg [1.1.3] - 2019-06-14 ^^^^^^^^^^^^^^^^^^^^ From bc61f206324f5280346bd8e7666971b1dc9beb49 Mon Sep 17 00:00:00 2001 From: RanaMostafa Date: Mon, 8 Jul 2019 21:21:12 +0200 Subject: [PATCH 10/10] Updated CHANGELOG to move rasa_train_command_fix under master unreleased instead of 1.14 --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 12fe22871bee..488f760280cd 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -36,6 +36,7 @@ Fixed - ``x in AnySlotDict`` is now ``True`` for any ``x``, which fixes empty slot warnings in interactive learning - ``rasa train`` now also includes NLU files in other formats than the Rasa format +- ``rasa train core`` no longer crashes without a ``--domain`` arg [1.1.4] - 2019-06-18 @@ -56,7 +57,6 @@ Fixed - fixed bug where facebook quick replies were not rendering - take FB quick reply payload rather than text as input - fixed bug where `training_data` path in `metadata.json` was an absolute path -- ``rasa train core`` no longer crashes without a ``--domain`` arg [1.1.3] - 2019-06-14 ^^^^^^^^^^^^^^^^^^^^