RasaHQ / rasa

💬 Open source machine learning framework to automate text- and voice-based conversations: NLU, dialogue management, connect to Slack, Facebook, and more - Create chatbots and voice assistants
https://rasa.com/docs/rasa/
Apache License 2.0
18.81k stars 4.62k forks source link

Rasa X Template API call design issues #4185

Closed rgstephens closed 2 years ago

rgstephens commented 5 years ago

Description of Problem: The template related API calls seem inconsistent and not completely thought out. This is related to the domain issue raised in #4176.

For example, if I call the update domain PUT call it will update intents and actions and leave response templates alone. However, this call has a store_templates option that probably shouldn't be there. There's a separate group of template API calls which seems like the place for all template calls.

However, if I call the update domain withstore_templates=true, I would expect it to only update response templates but instead it updates response templates, intents and actions.

There's also no DEL call for domains and templates so I have to use the PUT calls with null data.

Overview of the Solution: Remove all template related calls from the domain group of API calls.

In addition, complete the template API calls by adding the following:

sara-tagger commented 5 years ago

Thanks for submitting this feature request 🚀@wochinge will get back to you about it soon!✨

wochinge commented 5 years ago

Regarding the domain endpoint issues you are having: I think this is mostly a duplicate of https://github.com/RasaHQ/rasa/issues/4080, isn't it?

DEL call to delete all or selected templates

That's actually there, but not documented 🙈Thanks for pointing that out! Will do a pr for this asap.

Regarding the compatibility with yaml: I think the domain endpoint actually covers that already and json is basically yaml. However, I'll share it for discussion with the rest of the team.

rgstephens commented 5 years ago

Yes, there's overlap. I think this issue covers more than #4080 and that #4080 could be closed.

tmbo commented 5 years ago

I agree the domain and corresponding endpoints aren't very well thought through. I think the underlying issue is, that templates shouldn't be part of the domain in the first place as their configuration influences different parts of the pipeline (domain -> training, templates -> nlg, so prediction).

Having the templates be part of the domain makes it easier for users manually creating & maintaining the files as it is one file less to worry about. I think we should move towards a solution where we handle it similar to the nlu & core config: they can be part of the same file, but they do not need to be.

For the endpoints I think we should make sure:

rgstephens commented 5 years ago

I've upgraded to the latest Rasa X (0.20.2) and just noted a new issue with this endpoint, #4400.

This got me looking more closely at the /templates endpoints. These endpoints all work in json Rasa is setup for the user to specify the templates in markdown. Unlike the intents and stories, there's no convert method between markdown and json.

Will the /templates endpoints be supporting markdown?

rgstephens commented 5 years ago

I just noticed that @tmbo's comment about the /domain endpoint. I believe that should support the full CRUD calls not just RU. I want to be able to completely remove and reload a Rasa X configuration so I'd like to be able to Create and Delete the domain.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

alexweidauer commented 2 years ago

Thank you for raising this issue about Rasa X. We decided to stop supporting the Community Edition (free version) of ‘Rasa X’ (see more info here: https://rasa.com/blog/rasa-x-community-edition-changes/). That’s why we’ve closed your issue. We suggest that Rasa Enterprise customers raise the issue with our customer support team if they haven’t done so already.