Skip to content
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

Make Rasa X model pull interval configurable in local mode #4635

Merged
merged 46 commits into from
Nov 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
d061f06
first initial setup of the working change
Oct 18, 2019
31b953b
first initial commit of proposed changes
Oct 18, 2019
a117a06
Merge branch 'master' into issue_4524
btotharye Oct 18, 2019
41e73d4
fixing the logging message based on review
Oct 18, 2019
6b3123a
fixing logging feedback and if condition
Oct 18, 2019
1bf555e
updating from master
Oct 18, 2019
082a7d3
Merge branch 'issue_4524' of github.com:RasaHQ/rasa into issue_4524
Oct 18, 2019
522f15c
fixing kwargs to prevent key error
Oct 18, 2019
808f99d
fixed a typo in conditional statement and de-duplicated some code
Oct 18, 2019
235597b
added or condition to endpoints setup
Oct 18, 2019
06d0609
updated the changelog
Oct 18, 2019
e674b89
updating PR based on feedback
Oct 22, 2019
4c75164
fixing linting error
Oct 22, 2019
84d1c82
fixing changelog conflict
Oct 22, 2019
a8205b5
Merge branch 'master' into issue_4524
Oct 22, 2019
e94391d
Merge branch 'master' of github.com:RasaHQ/rasa into issue_4524
Oct 29, 2019
b6c2a50
updating based on feedback
Oct 29, 2019
7c8f638
Merge branch 'issue_4524' of github.com:RasaHQ/rasa into issue_4524
Oct 29, 2019
dcb9426
forgot check to verify custom_url isn't empty
Oct 29, 2019
0d9fd83
fixing 2 review issues
Oct 30, 2019
8685726
updating with black
Oct 30, 2019
b56a67b
updating first test to check for url
Nov 15, 2019
13a3e3e
updating tests and from master updates
Nov 15, 2019
54a372a
cleaning up some code based on review
Nov 15, 2019
0fc1032
fixed some formatting for tests
Nov 15, 2019
eb066cb
Merge branch 'master' into issue_4524
wochinge Nov 18, 2019
f4f640f
updated tests and setup warnings.warn
Nov 21, 2019
d3318a1
updated tests and setup warnings.warn
Nov 21, 2019
eac24ff
Merge branch 'master' into issue_4524
Nov 21, 2019
e966628
updating based on last review
Nov 22, 2019
e107e88
Merge branch 'issue_4524' of github.com:RasaHQ/rasa into issue_4524
Nov 22, 2019
d070e9f
Merge branch 'master' into issue_4524
Nov 22, 2019
7ed4742
Merge branch 'master' into issue_4524
wochinge Nov 25, 2019
e266dda
trying to fix changelog conflict issue
Nov 25, 2019
8d83388
fixing master merge conflict
Nov 25, 2019
ca437cb
fixing tests based on recent master changes
Nov 25, 2019
88b5d4f
fixed formatting of test file
Nov 25, 2019
212e2be
fixed formatting of test file
Nov 25, 2019
914aa4b
fixing changelog based on review
Nov 25, 2019
d27e3e0
Merge branch 'master' into issue_4524
Nov 26, 2019
3fde3fd
adding in updates to changelog
Nov 26, 2019
cd4df09
Merge branch 'issue_4524' of github.com:RasaHQ/rasa into issue_4524
Nov 26, 2019
32bf732
putting it on the right section this time
Nov 26, 2019
6c67039
fix duplicate `and`
wochinge Nov 26, 2019
c4d0379
removing duplicate and from changelog entry
Nov 26, 2019
4850add
removing duplicate and from changelog entry
Nov 26, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ Added

Changed
-------
- Print info message when running Rasa X and a custom model server url was specified in ``endpoints.yml``
- If a ``wait_time_between_pulls`` is configured for the model server in ``endpoints.yml``,
this will be used instead of the default one when running Rasa X

Removed
-------
Expand Down
25 changes: 20 additions & 5 deletions rasa/cli/x.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import asyncio
import importlib.util
import logging
import warnings
import os
import signal
import traceback
Expand Down Expand Up @@ -112,12 +113,27 @@ def _overwrite_endpoints_for_local_x(
from rasa.utils.endpoints import EndpointConfig
import questionary

# Checking if endpoint.yml has existing url and wait time values set, if so give
# warning we are overwriting the endpoint.yml file.
custom_wait_time_pulls = endpoints.model.kwargs.get("wait_time_between_pulls")
btotharye marked this conversation as resolved.
Show resolved Hide resolved
custom_url = endpoints.model.url
default_rasax_model_server_url = (
f"{rasa_x_url}/projects/default/models/tag/production"
)

if custom_url != default_rasax_model_server_url:
warnings.warn(
f"Ignoring url '{custom_url}' from 'endpoints.yml' and using "
f"'{default_rasax_model_server_url}' instead."
)

endpoints.model = EndpointConfig(
f"{rasa_x_url}/projects/default/models/tags/production",
default_rasax_model_server_url,
token=rasa_x_token,
wait_time_between_pulls=2,
wait_time_between_pulls=custom_wait_time_pulls or 2,
)

overwrite_existing_event_broker = False
if endpoints.event_broker and not _is_correct_event_broker(endpoints.event_broker):
cli_utils.print_error(
"Rasa X currently only supports a SQLite event broker with path '{}' "
Expand All @@ -132,9 +148,8 @@ def _overwrite_endpoints_for_local_x(
if not overwrite_existing_event_broker:
exit(0)

endpoints.event_broker = EndpointConfig(
type="sql", db=DEFAULT_EVENTS_DB, dialect="sqlite"
)
if not endpoints.tracker_store or overwrite_existing_event_broker:
endpoints.event_broker = EndpointConfig(type="sql", db=DEFAULT_EVENTS_DB)


def _is_correct_event_broker(event_broker: EndpointConfig) -> bool:
Expand Down
73 changes: 41 additions & 32 deletions tests/cli/test_rasa_x.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
from pathlib import Path
from unittest.mock import Mock
import warnings

from typing import Callable, Dict, Text, Any
import pytest
from typing import Callable, Dict
from _pytest.pytester import RunResult
from _pytest.monkeypatch import MonkeyPatch
import questionary
from _pytest.logging import LogCaptureFixture


from aioresponses import aioresponses

import rasa.utils.io as io_utils
from rasa.cli import x
from rasa.core.utils import AvailableEndpoints
from rasa.utils.endpoints import EndpointConfig
from rasa.core.utils import AvailableEndpoints


def test_x_help(run: Callable[..., RunResult]):
Expand Down Expand Up @@ -65,33 +65,6 @@ def test_prepare_credentials_if_already_valid(tmpdir: Path):
assert actual == credentials


@pytest.mark.parametrize(
"event_broker",
[
# Event broker was not configured.
{},
# Event broker was explicitly configured to work with Rasa X in local mode.
{"type": "sql", "dialect": "sqlite", "db": x.DEFAULT_EVENTS_DB},
# Event broker was configured but the values are not compatible for running Rasa
# X in local mode.
{"type": "sql", "dialect": "postgresql"},
],
)
def test_overwrite_endpoints_for_local_x(
event_broker: Dict[Text, Any], monkeypatch: MonkeyPatch
):
confirm = Mock()
confirm.return_value.ask.return_value = True
monkeypatch.setattr(questionary, "confirm", confirm)

event_broker_config = EndpointConfig.from_dict(event_broker)
endpoints = AvailableEndpoints(event_broker=event_broker_config)

x._overwrite_endpoints_for_local_x(endpoints, "test-token", "http://localhost:5002")

assert x._is_correct_event_broker(endpoints.event_broker)


def test_if_endpoint_config_is_valid_in_local_mode():
config = EndpointConfig(type="sql", dialect="sqlite", db=x.DEFAULT_EVENTS_DB)

Expand All @@ -111,6 +84,42 @@ def test_if_endpoint_config_is_invalid_in_local_mode(kwargs: Dict):
assert not x._is_correct_event_broker(config)


def test_overwrite_model_server_url():
endpoint_config = EndpointConfig(url="http://testserver:5002/models/default@latest")
endpoints = AvailableEndpoints(model=endpoint_config)
with pytest.warns(UserWarning):
x._overwrite_endpoints_for_local_x(endpoints, "test", "http://localhost")
assert (
endpoints.model.url == "http://localhost/projects/default/models/tag/production"
)


def test_reuse_wait_time_between_pulls():
btotharye marked this conversation as resolved.
Show resolved Hide resolved
test_wait_time = 5
endpoint_config = EndpointConfig(
url="http://localhost:5002/models/default@latest",
wait_time_between_pulls=test_wait_time,
)
endpoints = AvailableEndpoints(model=endpoint_config)
assert endpoints.model.kwargs["wait_time_between_pulls"] == test_wait_time


def test_default_wait_time_between_pulls():
endpoint_config = EndpointConfig(url="http://localhost:5002/models/default@latest")
endpoints = AvailableEndpoints(model=endpoint_config)
x._overwrite_endpoints_for_local_x(endpoints, "test", "http://localhost")
assert endpoints.model.kwargs["wait_time_between_pulls"] == 2


def test_default_model_server_url():
endpoint_config = EndpointConfig()
endpoints = AvailableEndpoints(model=endpoint_config)
x._overwrite_endpoints_for_local_x(endpoints, "test", "http://localhost")
assert (
endpoints.model.url == "http://localhost/projects/default/models/tag/production"
)


async def test_pull_runtime_config_from_server():
config_url = "http://example.com/api/config?token=token"
credentials = "rasa: http://example.com:5002/api"
Expand Down