-
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
Dialogflow Import Fix for Issue 6137 #6138
Conversation
Thanks for submitting a pull request 🚀 @melindaloubser1 will take a look at it as soon as possible ✨ |
Thanks for the PR! I don't have a dialogflow bot to test this with, but what happens now if the language is not defined in the config or on import? It still throws an error right? Edit: Sorry I just missed the linked issue! I'll look at that |
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.
Thanks for noticing this! For it to go into a patch, it needs to be based off of 1.10.x, not master - do you mind rebasing it?
@melindaloubser1 I recreated this as PR 6188. This way I don't have to worry about any previous merge conflicts that have already been resolved. |
Sure, that makes sense |
Proposed changes:
get_nlu_data
during trainingStatus (please check what you already did):
added some tests for the functionality
updated the documentation
updated the changelog (please check changelog for instructions)
reformat files using
black
(please check Readme for instructions)This is a fix for existing functionality which I tested against several dialogflow models.
No documentation are updates. The steps in current documentation work as expected now.