Cors support

[skreutzberger]

Proposed changes:

• Add CORS option to config as described in Add CORS option to config #478

Status:

• ready for code review
• there are tests for the functionality
• documentation updated
• changelog updated

[wrathagom]

@tmbo do I read the Travis stuff correct that this looks like it's just a timeout of some sorts. Can you manually kick off the tests again?

[PHLF]

Not sure about how to achieve it, but would it be possible to use CORS(app, resources={r"/*": {"origins": config['cors_origin']}}) instead?
With cors being a configuration setting accepting either a string or a list of URIs (strings).

[amn41]

This is great! Please also add a default value in rasa_nlu/config.py , see here

[skreutzberger]

@amn41 Do you want it default to True or False?

[PHLF]

I would rather like having the possibility to specify the allowed origin instead of using a wildcard.

[skreutzberger]

@PHLF I agree and it adds an important level of flexibility and security. I will directly use your cors_origins example. As default value I will forbid it (by using an empty list). For wildcard you can then still use ['*'] or go more restricted with ['*.mydomain.com', 'foo.bar.com']

I will update the PR in some minutes.

[PHLF]

Awesome! 👍

[tmbo]

Nice work! couple of minor changes and we are ready to merge 👍

[tmbo]

no need for the check, it will always be set (configuration is initialized with the defaults, so the key is always present)

[skreutzberger]

I am always doing that to avoid errors in the future. I am careful in expecting keys in a dict in all programming languages.

[tmbo]

right - and that is probably a good habit. There is a reason though we don't do it for the configuration values (and the reason why the configuration isn't simply a dict but a separate class): it ensures that every configuration key we expect to be present is present and set to a default value. This keeps all the default values in one place and avoids the need to handle unset values at every point the configuration is accessed.

[tmbo]

needs to be pinned to a version

[tmbo]

needs to be added to config_defaults.json as well (this will also fix the failing test)

[tmbo]

also please add the feature to the documentation so people can use it :)

• the configuration parameter needs to here: https://github.com/RasaHQ/rasa_nlu/blob/master/docs/config.rst
• and the cors feature should be mentioned on the server help page:
  https://github.com/RasaHQ/rasa_nlu/blob/master/docs/http.rst (probably as a new section next to Authentication)