-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
fix the "loading model" message which was logged twice #8278
Changes from 2 commits
2566827
5387545
3363140
117246b
6d069c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Fixed the 'loading model' message which was logged twice when using `rasa run`. |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -128,16 +128,19 @@ def should_retrain_nlu(self) -> bool: | |||||
return self.force_training or self.nlu | ||||||
|
||||||
|
||||||
def get_model(model_path: Text = DEFAULT_MODELS_PATH) -> TempDirectoryPath: | ||||||
"""Get a model and unpack it. Raises a `ModelNotFound` exception if | ||||||
no model could be found at the provided path. | ||||||
def get_local_model(model_path: Text = DEFAULT_MODELS_PATH) -> Text: | ||||||
"""Verifies that a model path exists. | ||||||
|
||||||
Args: | ||||||
model_path: Path to the zipped model. If it's a directory, the latest | ||||||
trained model is returned. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please also document the |
||||||
|
||||||
Returns: | ||||||
Path to the unpacked model. | ||||||
model_path: Path to the zipped model. If it's a directory, the latest | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
trained model is returned. | ||||||
|
||||||
Raises: | ||||||
ModelNotFound Exception: When no model could be found at the provided path. | ||||||
|
||||||
""" | ||||||
if not model_path: | ||||||
|
@@ -154,10 +157,30 @@ def get_model(model_path: Text = DEFAULT_MODELS_PATH) -> TempDirectoryPath: | |||||
elif not model_path.endswith(".tar.gz"): | ||||||
raise ModelNotFound(f"Path '{model_path}' does not point to a Rasa model file.") | ||||||
|
||||||
return model_path | ||||||
|
||||||
|
||||||
def get_model(model_path: Text = DEFAULT_MODELS_PATH) -> TempDirectoryPath: | ||||||
"""Get a model and unpack it. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
Args: | ||||||
model_path: Path to the zipped model. If it's a directory, the latest | ||||||
trained model is returned. | ||||||
|
||||||
Returns: | ||||||
Path to the unpacked model. | ||||||
|
||||||
Raises: | ||||||
ModelNotFound Exception: When no model could be found at the provided path. | ||||||
|
||||||
""" | ||||||
model_path = get_local_model(model_path) | ||||||
|
||||||
try: | ||||||
model_relative_path = os.path.relpath(model_path) | ||||||
except ValueError: | ||||||
model_relative_path = model_path | ||||||
|
||||||
logger.info(f"Loading model {model_relative_path}...") | ||||||
|
||||||
return unpack_model(model_path) | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,6 +42,7 @@ | |
can_finetune, | ||
create_package_rasa, | ||
get_latest_model, | ||
get_local_model, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minor thing: Could we please also import the things by their module here instead of importing the functions directly? |
||
get_model, | ||
get_model_subdirectories, | ||
model_fingerprint, | ||
|
@@ -78,6 +79,16 @@ def test_get_model_context_manager(trained_rasa_model: str): | |
assert not os.path.exists(unpacked) | ||
|
||
|
||
def test_get_local_model(trained_rasa_model: str): | ||
assert get_local_model(trained_rasa_model) == trained_rasa_model | ||
|
||
|
||
@pytest.mark.parametrize("model_path", ["foobar", "rasa", "README.md", None]) | ||
def test_get_local_model_exception(model_path: Optional[Text]): | ||
with pytest.raises(ModelNotFound): | ||
get_local_model(model_path) | ||
|
||
|
||
@pytest.mark.parametrize("model_path", ["foobar", "rasa", "README.md", None]) | ||
def test_get_model_exception(model_path: Optional[Text]): | ||
with pytest.raises(ModelNotFound): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to adapt that one, too. How about: