Ma77Ball commented on code in PR #4164:
URL: https://github.com/apache/texera/pull/4164#discussion_r2700293661
##########
amber/src/main/python/core/architecture/managers/test_executor_manager.py:
##########
@@ -57,26 +57,26 @@ def test_initialization(self, executor_manager):
assert executor_manager.executor_version == 0
def test_reject_r_tuple_language(self, executor_manager):
- """Test that 'r-tuple' language is rejected with AssertionError."""
- with pytest.raises(AssertionError) as exc_info:
+ """Test that 'r-tuple' language is rejected with RuntimeError when
plugin is not available."""
+ with pytest.raises(RuntimeError) as exc_info:
executor_manager.initialize_executor(
code=SAMPLE_OPERATOR_CODE, is_source=False, language="r-tuple"
)
- # Verify the error message mentions R UDF support has been dropped
- assert "not supported" in str(exc_info.value) or "dropped" in str(
+ # Verify the error message mentions R operators require the
texera-rudf package
+ assert "texera-rudf" in str(exc_info.value) or "R operators require"
in str(
exc_info.value
)
def test_reject_r_table_language(self, executor_manager):
- """Test that 'r-table' language is rejected with AssertionError."""
- with pytest.raises(AssertionError) as exc_info:
+ """Test that 'r-table' language is rejected with RuntimeError when
plugin is not available."""
+ with pytest.raises(RuntimeError) as exc_info:
executor_manager.initialize_executor(
code=SAMPLE_OPERATOR_CODE, is_source=False, language="r-table"
)
- # Verify the error message mentions R UDF support has been dropped
- assert "not supported" in str(exc_info.value) or "dropped" in str(
+ # Verify the error message mentions R operators require the
texera-rudf package
+ assert "texera-rudf" in str(exc_info.value) or "R operators require"
in str(
exc_info.value
)
Review Comment:
It could be good to add a test for when the plugin is present.
##########
amber/src/main/python/core/architecture/managers/executor_manager.py:
##########
@@ -132,13 +132,29 @@ class declaration.
:param language: The language of the operator code.
:return:
"""
- assert language not in [
- "r-tuple",
- "r-table",
- ], "R language is not supported by default. Please consult third party
plugin."
- executor: type(Operator) = self.load_executor_definition(code)
- self.executor = executor()
- self.executor.is_source = is_source
+ if language in ("r-tuple", "r-table"):
+ # R support is provided by an optional plugin (texera-rudf)
+ executor_type = "Tuple" if language == "r-tuple" else "Table"
+ try:
+ import texera_r
+
+ source_executor_class = getattr(
+ texera_r, f"R{executor_type}SourceExecutor"
+ )
+ executor_class = getattr(texera_r, f"R{executor_type}Executor")
+ except ImportError as e:
+ raise RuntimeError(
Review Comment:
Runtime Error is quite broad for logging; it might be useful to raise an
import error instead.
##########
amber/src/main/python/core/architecture/managers/executor_manager.py:
##########
@@ -132,13 +132,29 @@ class declaration.
:param language: The language of the operator code.
:return:
"""
- assert language not in [
- "r-tuple",
- "r-table",
- ], "R language is not supported by default. Please consult third party
plugin."
- executor: type(Operator) = self.load_executor_definition(code)
- self.executor = executor()
- self.executor.is_source = is_source
+ if language in ("r-tuple", "r-table"):
+ # R support is provided by an optional plugin (texera-rudf)
+ executor_type = "Tuple" if language == "r-tuple" else "Table"
+ try:
+ import texera_r
+
+ source_executor_class = getattr(
Review Comment:
I believe lines 141-144 could be refactored to resolve only one class
instead of both (the module required for execution: source_executor_class or
executor_class).
```
target_class_name = f"{executor_type}SourceExecutor" if is_source else
f"{executor_type}Executor"
executor_class = getattr(texera_r, target_class_name)
self.executor = executor_class(code)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
