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

don't schedule model pulling for testing #7684

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions changelog/7684.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Fixed a problem when using the `POST /model/test/intents` endpoint together with a
[model server](model-storage.mdx#load-model-from-server). The error looked as follows:

```
ERROR rasa.core.agent:agent.py:327 Could not load model due to Detected inconsistent loop usage. Trying to schedule a task on a new event loop, but scheduler was created with a different event loop. Make sure there is only one event loop in use and that the scheduler is running on that one.
```

This also fixes a problem where testing a model from a model server would change the
production model.
4 changes: 4 additions & 0 deletions rasa/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -1134,7 +1134,11 @@ async def _evaluate_model_using_test_set(
if model_path:
model_server = app.agent.model_server
if model_server is not None:
model_server = model_server.copy()
model_server.url = model_path
# Set wait time between pulls to `0` so that the agent does not schedule
# a job to pull the model from the server
model_server.kwargs["wait_time_between_pulls"] = 0
eval_agent = await _load_agent(
model_path, model_server, app.agent.remote_storage
)
Expand Down
110 changes: 81 additions & 29 deletions tests/test_server.py
Original file line number Diff line number Diff line change
@@ -1,48 +1,50 @@
import asyncio
import json
import os
import time
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I optimized the imports of the module using PyCharm 👀

import urllib.parse
import uuid
from contextlib import ExitStack
from http import HTTPStatus
from multiprocessing import Process, Manager
from multiprocessing.managers import DictProxy
from pathlib import Path
from unittest.mock import Mock, ANY
import requests
import time
import uuid
import urllib.parse

from typing import List, Text, Type, Generator, NoReturn, Dict, Optional
from contextlib import ExitStack
from unittest.mock import Mock, ANY

import pytest
import requests
from _pytest import pathlib
from _pytest.monkeypatch import MonkeyPatch
from aioresponses import aioresponses

import pytest
from freezegun import freeze_time
from mock import MagicMock
from multiprocessing import Process, Manager
from ruamel.yaml import StringIO
from sanic import Sanic
from sanic.testing import SanicASGITestClient

import rasa
import rasa.constants
import rasa.core.jobs
import rasa.nlu
import rasa.server
import rasa.shared.constants
import rasa.shared.utils.io
import rasa.utils.io
import rasa.server
import rasa.nlu
from rasa.core import utils
from rasa.core.tracker_store import InMemoryTrackerStore
from rasa.nlu.test import CVEvaluationResult
from rasa.shared.core import events
from rasa.core.agent import Agent
from rasa.core.agent import Agent, load_agent
from rasa.core.channels import (
channel,
CollectingOutputChannel,
RestInput,
SlackInput,
CallbackInput,
)
from rasa.train import TrainingResult
from rasa.core.channels.slack import SlackBot
from rasa.core.tracker_store import InMemoryTrackerStore
from rasa.model import unpack_model
from rasa.nlu.test import CVEvaluationResult
from rasa.shared.core import events
from rasa.shared.core.constants import (
ACTION_SESSION_START_NAME,
ACTION_LISTEN_NAME,
Expand All @@ -58,17 +60,12 @@
SessionStarted,
)
from rasa.shared.core.trackers import DialogueStateTracker
from rasa.model import unpack_model
from rasa.shared.nlu.constants import INTENT_NAME_KEY
from rasa.train import TrainingResult
from rasa.utils.endpoints import EndpointConfig
from sanic import Sanic
from sanic.testing import SanicASGITestClient

from tests.core.conftest import DEFAULT_STACK_CONFIG
from tests.nlu.utilities import ResponseTest
from tests.utilities import json_of_latest_request, latest_request
from ruamel.yaml import StringIO


# a couple of event instances that we can use for testing
test_events = [
Expand Down Expand Up @@ -122,6 +119,12 @@ def rasa_secured_app(rasa_server_secured: Sanic) -> SanicASGITestClient:
return rasa_server_secured.asgi_client


@pytest.fixture()
async def tear_down_scheduler() -> Generator[None, None, None]:
yield None
rasa.core.jobs.__scheduler = None


async def test_root(rasa_app: SanicASGITestClient):
_, response = await rasa_app.get("/")
assert response.status == HTTPStatus.OK
Expand Down Expand Up @@ -810,7 +813,7 @@ async def test_evaluate_intent_on_just_nlu_model(


async def test_evaluate_intent_with_model_param(
rasa_app: SanicASGITestClient, trained_nlu_model, default_nlu_data: Text
rasa_app: SanicASGITestClient, trained_nlu_model: Text, default_nlu_data: Text
):
_, response = await rasa_app.get("/status")
previous_model_file = response.json()["model_file"]
Expand All @@ -834,6 +837,59 @@ async def test_evaluate_intent_with_model_param(
assert previous_model_file == response.json()["model_file"]


async def test_evaluate_intent_with_model_server(
rasa_app: SanicASGITestClient,
trained_rasa_model: Text,
default_nlu_data: Text,
tear_down_scheduler: None,
):
production_model_server_url = (
"https://example.com/webhooks/actions?model=production"
)
test_model_server_url = "https://example.com/webhooks/actions?model=test"

nlu_data = rasa.shared.utils.io.read_file(default_nlu_data)

with aioresponses() as mocked:
# Mock retrieving the production model from the model server
mocked.get(
production_model_server_url,
body=Path(trained_rasa_model).read_bytes(),
headers={"ETag": "production"},
)
# Mock retrieving the test model from the model server
mocked.get(
test_model_server_url,
body=Path(trained_rasa_model).read_bytes(),
headers={"ETag": "test"},
)

agent_with_model_server = await load_agent(
model_server=EndpointConfig(production_model_server_url)
)
rasa_app.app.agent = agent_with_model_server

_, response = await rasa_app.post(
f"/model/test/intents?model={test_model_server_url}",
data=nlu_data,
headers={"Content-type": rasa.server.YAML_CONTENT_TYPE},
)

assert response.status == HTTPStatus.OK
assert set(response.json().keys()) == {
"intent_evaluation",
"entity_evaluation",
"response_selection_evaluation",
}

production_model_server = rasa_app.app.agent.model_server
# Assert that the model server URL for the test didn't override the production
# model server URL
assert production_model_server.url == production_model_server_url
# Assert the tests didn't break pulling the models
assert production_model_server.kwargs.get("wait_time_between_pulls") != 0


async def test_cross_validation(
rasa_app_nlu: SanicASGITestClient, default_nlu_data: Text
):
Expand Down Expand Up @@ -1348,7 +1404,7 @@ async def test_load_model(rasa_app: SanicASGITestClient, trained_core_model: Tex


async def test_load_model_from_model_server(
rasa_app: SanicASGITestClient, trained_core_model: Text
rasa_app: SanicASGITestClient, trained_core_model: Text, tear_down_scheduler: None
):
_, response = await rasa_app.get("/status")

Expand Down Expand Up @@ -1380,10 +1436,6 @@ async def test_load_model_from_model_server(

assert old_fingerprint != response.json()["fingerprint"]

import rasa.core.jobs

rasa.core.jobs.__scheduler = None


async def test_load_model_invalid_request_body(rasa_app: SanicASGITestClient):
_, response = await rasa_app.put("/model")
Expand Down