Repository: RasaHQ/rasa
Pull request: Fixed crash when --domain not provided for rasa train command
Fixes #3794

**Proposed changes**:
- 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
- One more additional check added to `--domain` argument

**Tests**:
- [CORE] Provide Invalid domain file path  **[Error Message Displayed]**
`python rasa/rasa train core --domain rasa/examples/formbot/domain1.yml  --config rasa/examples/formbot/config.yml --stories rasa/examples/formbot/data/ --out rasa/examples/formbot/models --augmentation 0 --quiet`

- [CORE] No domain file provided **[Error Message Displayed]**
`python rasa/rasa train core  --config rasa/examples/formbot/config.yml --stories rasa/examples/formbot/data/ --out rasa/examples/formbot/models --augmentation 0 --quiet`

- [CORE] Provided valid domain file **[PASS]**
`python rasa/rasa train core --domain rasa/examples/formbot/domain.yml  --config rasa/examples/formbot/config.yml --stories rasa/examples/formbot/data/ --out rasa/examples/formbot/models --augmentation 0 --quiet`

- [NLU] No domain file provided **[PASS]**
`python rasa/rasa train nlu  --config rasa/examples/formbot/config.yml --nlu rasa/examples/formbot/data/ --out rasa/examples/formbot/models`

**Status (please check what you already did)**:
- [x] made PR ready for code review
- [x] added some tests for the functionality
- [ ] updated the documentation
- [x] updated the changelog
- [x] reformat files using `black` (please check [Readme](https://github.com/RasaHQ/rasa_nlu#code-style) for instructions)


Problem statement:
rasa train command error message
**Rasa version**: 1.1.13

**Issue**:

If you run the `rasa train` command and don't provide a `--domain` option, a python stack trace results instead of an error message message that the domain file was a required option.

**Error (including full traceback)**:
```
docker run -v $(pwd)/data:/app/data rasa/rasa:latest-full train core --config /app/data/config.yml --stories /app/data/stories --out /app/data/models --augmentation 0 --quiet
2019-06-15 00:52:47 ERROR    rasa.core.training.dsl  - Error in line 2: 'NoneType' object has no attribute 'intents'
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/rasa/core/training/dsl.py", line 301, in process_lines
    await self.add_user_messages(user_messages, line_num)
  File "/usr/local/lib/python3.6/site-packages/rasa/core/training/dsl.py", line 383, in add_user_messages
    *[self._parse_message(m, line_num) for m in messages]
  File "/usr/local/lib/python3.6/site-packages/rasa/core/training/dsl.py", line 367, in _parse_message
    if intent_name not in self.domain.intents:
AttributeError: 'NoneType' object has no attribute 'intents'
2019-06-15 00:52:47 ERROR    rasa.core.training.dsl  - Invalid story file format. Failed to parse '/tmp/tmpz84lkggv/c24e99a4b7ff480f80047ec55ce5b484_claimzen.md'
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/rasa/core/training/dsl.py", line 301, in process_lines
    await self.add_user_messages(user_messages, line_num)
  File "/usr/local/lib/python3.6/site-packages/rasa/core/training/dsl.py", line 383, in add_user_messages
    *[self._parse_message(m, line_num) for m in messages]
  File "/usr/local/lib/python3.6/site-packages/rasa/core/training/dsl.py", line 367, in _parse_message
    if intent_name not in self.domain.intents:
AttributeError: 'NoneType' object has no attribute 'intents'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/rasa/core/training/dsl.py", line 214, in read_from_file
    return await reader.process_lines(lines)
  File "/usr/local/lib/python3.6/site-packages/rasa/core/training/dsl.py", line 313, in process_lines
    raise ValueError(msg)
ValueError: Error in line 2: 'NoneType' object has no attribute 'intents'
Training Core model...
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/rasa/core/training/dsl.py", line 301, in process_lines
    await self.add_user_messages(user_messages, line_num)
  File "/usr/local/lib/python3.6/site-packages/rasa/core/training/dsl.py", line 383, in add_user_messages
    *[self._parse_message(m, line_num) for m in messages]
  File "/usr/local/lib/python3.6/site-packages/rasa/core/training/dsl.py", line 367, in _parse_message
    if intent_name not in self.domain.intents:
AttributeError: 'NoneType' object has no attribute 'intents'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/bin/rasa", line 10, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.6/site-packages/rasa/__main__.py", line 70, in main
    cmdline_arguments.func(cmdline_arguments)
  File "/usr/local/lib/python3.6/site-packages/rasa/cli/train.py", line 121, in train_core
    kwargs=extract_additional_arguments(args),
  File "/usr/local/lib/python3.6/site-packages/rasa/train.py", line 215, in train_core
    kwargs=kwargs,
  File "uvloop/loop.pyx", line 1451, in uvloop.loop.Loop.run_until_complete
  File "/usr/local/lib/python3.6/site-packages/rasa/train.py", line 277, in train_core_async
    kwargs=kwargs,
  File "/usr/local/lib/python3.6/site-packages/rasa/train.py", line 303, in _train_core_with_validated_data
    kwargs=kwargs,
  File "/usr/local/lib/python3.6/site-packages/rasa/core/train.py", line 57, in train
    stories_file, exclusion_percentage=exclusion_percentage, **data_load_args
  File "/usr/local/lib/python3.6/site-packages/rasa/core/agent.py", line 616, in load_data
    exclusion_percentage=exclusion_percentage,
  File "/usr/local/lib/python3.6/site-packages/rasa/core/training/__init__.py", line 50, in load_data
    resource_name, domain, exclusion_percentage=exclusion_percentage
  File "/usr/local/lib/python3.6/site-packages/rasa/core/training/__init__.py", line 29, in extract_story_graph
    exclusion_percentage=exclusion_percentage,
  File "/usr/local/lib/python3.6/site-packages/rasa/core/training/dsl.py", line 186, in read_from_folder
    f, domain, interpreter, template_variables, use_e2e
  File "/usr/local/lib/python3.6/site-packages/rasa/core/training/dsl.py", line 214, in read_from_file
    return await reader.process_lines(lines)
  File "/usr/local/lib/python3.6/site-packages/rasa/core/training/dsl.py", line 313, in process_lines
    raise ValueError(msg)
```

**Command or request that led to error**:
```
docker run -v $(pwd)/data:/app/data rasa/rasa:latest-full train core --config /app/data/config.yml --stories /app/data/stories --out /app/data/models --augmentation 0 --quiet
```

Hints from issue discussion:
Thanks for raising this issue, @ricwo  will get back to you about it soon.
Hi @rgstephens, we should indeed throw a more useful warning and not just fail. Thanks for pointing this out! 
Hello @ricwo ,
I am currently working on the issue. 
I just have one question :
- Should it fires just a warning and continues execution? or it should throw an error and then terminates program ?
Hi @RanaMostafaAbdElMohsen, thanks for working on this! Training can continue in that case, but only for NLU. The warning should explicitly state that Core training is skipped because no domain was found.
@RanaMostafaAbdElMohsen thanks a lot for taking this on, let us know if you have any questions in the process
@akelad @ricwo 
Would you please help me  in test coverage ?
I think a new test case should be added to `rasa/tests/cli/test_rasa_train.py` as below:
```python
def test_train_core_no_domain(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 os.path.exists("train_rasa_models/rasa-model.tar.gz")
    assert os.path.isfile("train_rasa_models/rasa-model.tar.gz")
```
Am I on the right track ? Please advise
@RanaMostafaAbdElMohsen, this test is indeed necessary! It would be great if we could also check for `domain.yml` is not present. To do this, we'll have to unpack the existing model first:


```python
from rasa import model

trained_model_path = "train_rasa_models/rasa-model.tar.gz"
unpacked = model.unpack_model(trained_model_path)

# finterprint.json is present
fingerprint_path = os.path.join(unpacked, "fingerprint.json")
assert os.path.exists(fingerprint_path)

# core/domain.yml is not present
domain_path = os.path.join(unpacked, "core", "domain.yml")
assert not os.path.exists(domain_path)
```

we should probably also check that NLU training worked, by checking for the presence of, for example, `nlu/metadata.json`.
@ricwo @akelad 
I have followed documentation steps so I can run tests locally using `make tests` command
and a large number of test cases fail locally. However, when test cases execute on Travis, they run successfully.

Can you please advise what am I missing here so tests execute correctly, so I can validate my own test cases.
Thanks a lot. 
@RanaMostafaAbdElMohsen did you install all requirements in `requirements-dev.txt`? If that doesn't help, you might have to try with a fresh python virtual / conda environment.

would running only your newly added tests instead of the whole suite be an option?

Commit message: Fix linting error

Patch to review:
diff --git a/rasa/train.py b/rasa/train.py
index 018694e89755..112d5fe9916f 100644
--- a/rasa/train.py
+++ b/rasa/train.py
@@ -71,12 +71,8 @@ async def train_async(
     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)
-        )
-        return None
+    except InvalidDomain:
+        domain = None
 
     story_directory, nlu_data_directory = data.get_core_nlu_directories(
         training_files, skill_imports
@@ -87,6 +83,11 @@ async def train_async(
         nlu_data = stack.enter_context(TempDirectoryPath(nlu_data_directory))
         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 await _train_async_internal(
             domain,
             config,
@@ -99,6 +100,26 @@ async def train_async(
             kwargs,
         )
 
+    if domain is None:
+        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
+):
+    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,
+    )
+
 
 async def _train_async_internal(
     domain: Union[Domain, Text],
@@ -293,16 +314,15 @@ async def train_core_async(
 
     skill_imports = SkillSelector.load(config, stories)
 
-    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
+    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
 
     train_context = TempDirectoryPath(data.get_core_directory(stories, skill_imports))
 
diff --git a/tests/cli/test_rasa_train.py b/tests/cli/test_rasa_train.py
index f6db7e159bcb..e483095b82bc 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,
@@ -37,6 +39,32 @@ def test_train(run_in_default_project):
     assert os.path.basename(files[0]) == "test-model.tar.gz"
 
 
+def test_train_no_domain_exists(run_in_default_project):
+
+    os.remove("domain.yml")
+    run_in_default_project(
+        "train",
+        "-c",
+        "config.yml",
+        "--data",
+        "data",
+        "--out",
+        "train_models_no_domain",
+        "--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)
+
+
 def test_train_skip_on_model_not_changed(run_in_default_project):
     temp_dir = os.getcwd()
 
@@ -118,6 +146,28 @@ 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):
+
+    os.remove("domain.yml")
+    run_in_default_project(
+        "train",
+        "core",
+        "--config",
+        "config.yml",
+        "--domain",
+        "domain1.yml",
+        "--stories",
+        "data",
+        "--out",
+        "train_rasa_models_no_domain",
+        "--fixed-model-name",
+        "rasa-model",
+    )
+
+    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()):


Code review comments:
diff_hunk: @@ -99,6 +100,26 @@ def train(
             kwargs,
         )
 
+    if domain is None:
+        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
+):
+    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"
path: rasa/train.py
text: @user1:
```suggestion
        "Please specify a valid domain using '--domain' argument or check if the provided domain file exists."
```

@author:
Resolved

diff_hunk: @@ -99,6 +100,26 @@ def train(
             kwargs,
         )
 
+    if domain is None:
+        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
+):
+    print_warning(
+        "Core training is skipped because no domain was found. "
path: rasa/train.py
text: @user1:
```suggestion
        "Core training was skipped because no valid domain file was found. "
```

@user1:
perhaps make it explicit here that a NLU-only model was created. 

@author:
Resolved

diff_hunk: @@ -99,6 +100,26 @@ def train(
             kwargs,
         )
 
+    if domain is None:
+        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
+):
+    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(
path: rasa/train.py
text: @user1:
Can we make the warning message show up after the NLU training? I worry that it won't be seen if the training pipeline is long or with lots of epochs. I think storing this model here as a variable, then doing the print_warning, then returning the variable should do it.

@author:
Resolved