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.

Sign up for GitHub

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

Jump to bottom

don't schedule model pulling for testing #7684

Closed
wants to merge 4 commits into from
Closed

don't schedule model pulling for testing #7684

wants to merge 4 commits into from

Conversation

wochinge
Copy link
Contributor

@wochinge wochinge commented Jan 6, 2021 •
edited
Loading

Proposed changes:

  • copy the model server config before adapting it to avoid that testing a model from a model server affects the location where the production model is pulled from
  • don't schedule model pulling for model tests through the API which get a model server URL. This avoid the following error:
    Screenshot 2021-01-06 at 14 22 38

Status (please check what you already did):

  • added some tests for the functionality
  • updated the documentation
  • updated the changelog (please check changelog for instructions)
  • reformat files using black (please check Readme for instructions)

wochinge
wochinge commented Jan 6, 2021
View reviewed changes
tests/test_server.py
@@ -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 👀

@wochinge wochinge requested a review from federicotdn January 6, 2021 13:57
@wochinge wochinge marked this pull request as ready for review January 6, 2021 13:57
federicotdn
federicotdn approved these changes Jan 6, 2021
View reviewed changes
Copy link
Contributor

@federicotdn federicotdn left a comment

Choose a reason for hiding this comment

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

Did a quick test with Rasa X on local mode and everything looks good 👍

@wochinge wochinge mentioned this pull request Jan 6, 2021
Merged
4 tasks
@wochinge
Copy link
Contributor Author

wochinge commented Jan 6, 2021

closing in favor of a combined merge here #7688

@wochinge wochinge closed this Jan 6, 2021
@wochinge wochinge deleted the test-model-bugfix branch January 6, 2021 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@federicotdn federicotdn federicotdn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wochinge @federicotdn