search

RasaHQ / financial-demo

A demo for a financial services bot
Apache License 2.0
309 stars 398 forks source link

Added required_slots field to form definitions. #146

Closed kedz closed 3 years ago

kedz commented 3 years ago

Currently the form definitions in the domain.yml do not use the required_slots key. Not using the key was marked as deprecated in Rasa 2.6. As we move to 3.0 and start removing deprecated behaviors, not using the required_slots keys will cause training to fail on YAML validation.

This PR adds the required_slots key to all form definitions in the domain.yml. This should not change the behavior of the bot.

ArjaanBuijk commented 3 years ago

@kedz , The training shows errors about missing slots in the domain file. https://github.com/RasaHQ/financial-demo/runs/3104521051?check_suite_focus=true Are we forgetting something?

kedz commented 3 years ago

@ArjaanBuijk I'm able to run training and testing locally on rasa/main. What version is the CI pipeline running?

ArjaanBuijk commented 3 years ago

@kedz , rasa_enterprise_version : 0.38.1 rasa_version : 2.4.0-spacy-en rasa_sdk_version : 2.4.0

What version are you running?

I think upgrading to latest will be good.

Shall I take care of that?

kedz commented 3 years ago

Thanks @ArjaanBuijk! That would be great if you could update the CI version (I'm not sure how to do it myself) I just tested out the latest rasa os release branch (2.8.x) and it seems to be working on that version.

ArjaanBuijk commented 3 years ago

@kedz , I can confirm it is not your update that is causing the failure. I am digging into it to find out where the issue is.

ArjaanBuijk commented 3 years ago

@kedz , everything is working fine with these versions:

Checks passed, and I will merge it into main.