-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ATO 490] Rasa train doesn't throw an error if form in slot mappings active loop doesn't exist #12324
[ATO 490] Rasa train doesn't throw an error if form in slot mappings active loop doesn't exist #12324
Conversation
…-rasa-train-doesnt-throw-an-error-if-form-in-slot-mappings-active-loop-doesnt-exist
…esnt-throw-an-error-if-form-in-slot-mappings-active-loop-doesnt-exist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great 🎉 Left some suggestions for improvements.
I'd also recommend outlining this solution to Product and CSE to get their stamp of approval (for the overall solution of giving flexibility to run validation checks via additional flags).
…esnt-throw-an-error-if-form-in-slot-mappings-active-loop-doesnt-exist
…esnt-throw-an-error-if-form-in-slot-mappings-active-loop-doesnt-exist
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏻 All my previous comments have been addressed, I only asked a clarifying question on a unit test.
…esnt-throw-an-error-if-form-in-slot-mappings-active-loop-doesnt-exist
docs/docs/command-line-interface.mdx
Outdated
@@ -135,6 +135,10 @@ these, `rasa train` will fall back to one of these commands by default. | |||
The name of the model by default is `<timestamp>.tar.gz`. If you want to name your model differently, | |||
you can specify the name using the `--fixed-model-name` flag. | |||
|
|||
By default validation is run before training the model. If you want to skip validation, you can use the `--skip-validation` flag. | |||
If you want to fail on validation warnings, you can use the `--fail-on-validation-warnings` flag. | |||
The --validation-max-history is analogous to the `--max-history` argument of `rasa data validate`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The --validation-max-history is analogous to the `--max-history` argument of `rasa data validate`. | |
The `--validation-max-history` is analogous to the `--max-history` argument of `rasa data validate`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
➕
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great 💯 Only two tiny comments that should be addressed before merging.
docs/docs/command-line-interface.mdx
Outdated
@@ -135,6 +135,10 @@ these, `rasa train` will fall back to one of these commands by default. | |||
The name of the model by default is `<timestamp>.tar.gz`. If you want to name your model differently, | |||
you can specify the name using the `--fixed-model-name` flag. | |||
|
|||
By default validation is run before training the model. If you want to skip validation, you can use the `--skip-validation` flag. | |||
If you want to fail on validation warnings, you can use the `--fail-on-validation-warnings` flag. | |||
The --validation-max-history is analogous to the `--max-history` argument of `rasa data validate`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
➕
🚀 A preview of the docs have been deployed at the following URL: https://12324--rasahq-docs-rasa-v2.netlify.app/docs/rasa |
Proposed changes:
--validate-before-training
,--fail-on-validation-warnings
and--validation-max-history
torasa train
to bring it inline withrasa data validate
Status (please check what you already did):
black
(please check Readme for instructions)