[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #12135: [DSIP-13][python]Sub problem: Add resource plugin support to task python, dataX, CustomDataX and Sql

[GitBox <gi...@apache.org>]

zhongjiajie commented on code in PR #12135:
URL: https://github.com/apache/dolphinscheduler/pull/12135#discussion_r993185618


##########
dolphinscheduler-python/pydolphinscheduler/tests/tasks/test_sql.py:
##########
@@ -16,13 +16,28 @@
 # under the License.
 
 """Test Task Sql."""
-
-
+from pathlib import Path
 from unittest.mock import patch
 
 import pytest
 
+from pydolphinscheduler.resources_plugin import Local
 from pydolphinscheduler.tasks.sql import Sql, SqlType
+from pydolphinscheduler.utils import file
+from tests.testing.file import delete_file
+
+file_name = "local_res.sql"
+file_content = 'echo "test res_local"'

Review Comment:
   nit, but can we change content to SQL query instead of shell?



##########
dolphinscheduler-python/pydolphinscheduler/src/pydolphinscheduler/tasks/python.py:
##########
@@ -55,48 +55,51 @@ def foo():
         want to execute.
     """
 
-    _task_custom_attr = {
-        "raw_script",
-    }
+    _task_custom_attr = {"raw_script", "definition"}
+
+    ext: set = {".py"}
+    ext_attr: Union[str, types.FunctionType] = "_definition"
 
     def __init__(
         self, name: str, definition: Union[str, types.FunctionType], *args, **kwargs
     ):
+        self._definition = definition
         super().__init__(name, TaskType.PYTHON, *args, **kwargs)
-        self.definition = definition
 
     def _build_exe_str(self) -> str:
         """Build executable string from given definition.
 
         Attribute ``self.definition`` almost is a function, we need to call this function after parsing it
         to string. The easier way to call a function is using syntax ``func()`` and we use it to call it too.
         """
-        if isinstance(self.definition, types.FunctionType):
-            py_function = inspect.getsource(self.definition)
-            func_str = f"{py_function}{self.definition.__name__}()"
+        definition = getattr(self, "definition")
+        if isinstance(definition, types.FunctionType):
+            py_function = inspect.getsource(definition)
+            func_str = f"{py_function}{definition.__name__}()"

Review Comment:
   It is same as `definition = getattr(self, "definition")`



##########
dolphinscheduler-python/pydolphinscheduler/src/pydolphinscheduler/tasks/python.py:
##########
@@ -55,48 +55,51 @@ def foo():
         want to execute.
     """
 
-    _task_custom_attr = {
-        "raw_script",
-    }
+    _task_custom_attr = {"raw_script", "definition"}
+
+    ext: set = {".py"}
+    ext_attr: Union[str, types.FunctionType] = "_definition"
 
     def __init__(
         self, name: str, definition: Union[str, types.FunctionType], *args, **kwargs
     ):
+        self._definition = definition
         super().__init__(name, TaskType.PYTHON, *args, **kwargs)
-        self.definition = definition
 
     def _build_exe_str(self) -> str:
         """Build executable string from given definition.
 
         Attribute ``self.definition`` almost is a function, we need to call this function after parsing it
         to string. The easier way to call a function is using syntax ``func()`` and we use it to call it too.
         """
-        if isinstance(self.definition, types.FunctionType):
-            py_function = inspect.getsource(self.definition)
-            func_str = f"{py_function}{self.definition.__name__}()"
+        definition = getattr(self, "definition")
+        if isinstance(definition, types.FunctionType):
+            py_function = inspect.getsource(definition)
+            func_str = f"{py_function}{definition.__name__}()"

Review Comment:
   we can directly use `self.definition` here



##########
dolphinscheduler-python/pydolphinscheduler/src/pydolphinscheduler/tasks/python.py:
##########
@@ -55,48 +55,51 @@ def foo():
         want to execute.
     """
 
-    _task_custom_attr = {
-        "raw_script",
-    }
+    _task_custom_attr = {"raw_script", "definition"}
+
+    ext: set = {".py"}
+    ext_attr: Union[str, types.FunctionType] = "_definition"
 
     def __init__(
         self, name: str, definition: Union[str, types.FunctionType], *args, **kwargs
     ):
+        self._definition = definition
         super().__init__(name, TaskType.PYTHON, *args, **kwargs)
-        self.definition = definition
 
     def _build_exe_str(self) -> str:
         """Build executable string from given definition.
 
         Attribute ``self.definition`` almost is a function, we need to call this function after parsing it
         to string. The easier way to call a function is using syntax ``func()`` and we use it to call it too.
         """
-        if isinstance(self.definition, types.FunctionType):
-            py_function = inspect.getsource(self.definition)
-            func_str = f"{py_function}{self.definition.__name__}()"
+        definition = getattr(self, "definition")
+        if isinstance(definition, types.FunctionType):
+            py_function = inspect.getsource(definition)
+            func_str = f"{py_function}{definition.__name__}()"

Review Comment:
   we do you change those, does it raise error?



-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org