Check for missing intent value in stories and rules and raise exception #8648
Conversation
changelog/8587.bugfix.md
Outdated
| @@ -0,0 +1 @@ | |||
| Catch AttributeError in YamlStoryReader method when intent value is missing in stories or rules and re-raise as RasaException. | |||
There was a problem hiding this comment.
optional: Users probably don't know the YAMLStoryReaderr so I'd not expose the information to them and make it a bit easier to understand.
| Catch AttributeError in YamlStoryReader method when intent value is missing in stories or rules and re-raise as RasaException. | |
| Catch `AttributeError` when reading stories in YAML format when intent value is missing in stories or rules and re-raise as `RasaException`. |
There was a problem hiding this comment.
should this really be a misc changelog as it doesn't really affect the users?
|
|
||
|
|
||
| def test_raises_exception_missing_intent_in_rules(domain: Domain): | ||
| rules = "data/test_yaml_stories/rules_missing_intent.yml" |
There was a problem hiding this comment.
optional: I'd slightly prefer to inline the stories because it makes the tests easier to read (I don't have to jump to the file to understand the test
There was a problem hiding this comment.
I'll remember this for future tests 👍
| assert "Missing intent value" in str(e.value) | ||
|
|
||
|
|
||
| def test_raises_exception_missing_intent_in_stories(domain: Domain): |
There was a problem hiding this comment.
how about using pytest.mark.parametrize for these two?
| try: | ||
| user_intent = step.get(KEY_USER_INTENT, "").strip() | ||
| except AttributeError as e: | ||
| raise RasaException( | ||
| f"Missing intent value in {self._get_item_title()} step: {step} " | ||
| f"in '{self.source_name}'" | ||
| ) from e |
There was a problem hiding this comment.
should we actually raise or treat this the same as the warning a few lines ahead? We don't raise for any other case so keeping it a warning seems more consistent to me
There was a problem hiding this comment.
True, I can change that - the question I have, when I use raise_warning would the next lines of code execute or do I need to add an empty return statement to stop the execution once this warning was raised?
There was a problem hiding this comment.
The next lines of code would execute. raise_warning should probably rather be something like print_warning to avoid that confusion
| except AttributeError: | ||
| rasa.shared.utils.io.raise_warning( | ||
| f"Issue found in '{self.source_name}':\n" | ||
| f"Missing intent value in {self._get_item_title()} step: {step} ." |
There was a problem hiding this comment.
how about adding a link to the docs?
Proposed changes:
stories.ymlorrules.ymlhave no value #8587Status (please check what you already did):
black(please check Readme for instructions)