Repository: RasaHQ/rasa
Pull request: Turned `utils.common` deprecation warning into exception
**Proposed changes**:
- fixes #8873 

**Status (please check what you already did)**:
- [x] reformat files using `black` (please check [Readme](https://github.com/RasaHQ/rasa#code-style) for instructions)


Problem statement:
3.0 Deprecation Warning: Convert warning into exception in `rasa.shared.utils.common.py`
**Background**
This warning was found in `rasa.shared.utils.common.py:L48`
```
"f""`class_from_module_path()` is expected to return a class, ""
f""but {module_path} is not one. ""
f""This warning will be converted ""
f""into an exception in {NEXT_MAJOR_VERSION_FOR_DEPRECATIONS}."""
```

**Definition of Done**
- [ ] resolve warning in 3.0

Hints from issue discussion:

Commit message: Merge branch 'main' into 8873-utils-common-deprecations

Patch to review:
diff --git a/rasa/shared/utils/common.py b/rasa/shared/utils/common.py
index 0a46e87175b4..5ee91e70e2c1 100644
--- a/rasa/shared/utils/common.py
+++ b/rasa/shared/utils/common.py
@@ -5,10 +5,6 @@
 import logging
 from typing import Text, Dict, Optional, Any, List, Callable, Collection
 
-import rasa.shared.utils.io
-from rasa.shared.constants import NEXT_MAJOR_VERSION_FOR_DEPRECATIONS
-
-
 logger = logging.getLogger(__name__)
 
 
@@ -45,13 +41,10 @@ def class_from_module_path(
         raise ImportError(f"Cannot retrieve class from path {module_path}.")
 
     if not inspect.isclass(klass):
-        rasa.shared.utils.io.raise_deprecation_warning(
+        raise TypeError(
             f"`class_from_module_path()` is expected to return a class, "
-            f"but {module_path} is not one. "
-            f"This warning will be converted "
-            f"into an exception in {NEXT_MAJOR_VERSION_FOR_DEPRECATIONS}."
+            f"but for {module_path} we got a {type(klass)}."
         )
-
     return klass
 
 
diff --git a/tests/shared/utils/test_common.py b/tests/shared/utils/test_common.py
index dfd507b0604f..c8db3ac6cb79 100644
--- a/tests/shared/utils/test_common.py
+++ b/tests/shared/utils/test_common.py
@@ -1,9 +1,8 @@
 import asyncio
-from typing import Any, Collection, List, Optional, Text
+from typing import Collection, List, Optional, Text
 from unittest.mock import Mock
 
 import pytest
-from _pytest.recwarn import WarningsRecorder
 
 import rasa.shared.core.domain
 import rasa.shared.utils.common
@@ -175,17 +174,7 @@ def test_class_from_module_path_not_found(
         rasa.shared.utils.common.class_from_module_path(module_path, lookup_path)
 
 
-@pytest.mark.parametrize(
-    "module_path, result, outcome",
-    [
-        ("rasa.shared.core.domain.Domain", rasa.shared.core.domain.Domain, True),
-        ("rasa.shared.core.domain.logger", rasa.shared.core.domain.logger, False),
-    ],
-)
-def test_class_from_module_path_ensure_class(
-    module_path: Text, outcome: bool, result: Any, recwarn: WarningsRecorder
-):
-    klass = rasa.shared.utils.common.class_from_module_path(module_path)
-    assert klass is result
-
-    assert bool(len(recwarn)) is not outcome
+def test_class_from_module_path_fails():
+    module_path = "rasa.shared.core.domain.logger"
+    with pytest.raises(TypeError):
+        rasa.shared.utils.common.class_from_module_path(module_path)


Code review comments:
diff_hunk: @@ -45,13 +41,10 @@ def class_from_module_path(
         raise ImportError(f"Cannot retrieve class from path {module_path}.")
 
     if not inspect.isclass(klass):
-        rasa.shared.utils.io.raise_deprecation_warning(
+        raise TypeError(
path: rasa/shared/utils/common.py
text: @user1:
I'm wondering if it's not better to raise a `RasaException` here, wouldn't this be an usage error? 🤔 
Also the docstring should be updated with the 2nd error raised other than `ImportError`.

@author:
Good point, RasaException does not trigger the telemetry. I guess then we would want to remove the ImportError as well

@user1:
Maybe it's best to revert this change for `ImportError` based on the latest failed test information 😅 
I'm actually reviewing a PR that is filtering `ImportError` exceptions from being sent to Sentry so that will be taken care of too indirectly.

@author:
😅 agreed