[GitHub] [airflow] uranusjr commented on a diff in pull request #25780: Implement ExternalPythonOperator

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

uranusjr commented on code in PR #25780:
URL: https://github.com/apache/airflow/pull/25780#discussion_r948601230


##########
airflow/decorators/__init__.pyi:
##########
@@ -124,6 +126,40 @@ class TaskDecoratorCollection:
     @overload
     def virtualenv(self, python_callable: Callable[FParams, FReturn]) -> Task[FParams, FReturn]: ...
     @overload
+    def external_python(
+        self,
+        *,
+        python_fspath: str = None,

Review Comment:
   I wonder if we should just call this argument `python`. Also this probably should not have `= None`.



##########
airflow/decorators/__init__.pyi:
##########
@@ -124,6 +126,40 @@ class TaskDecoratorCollection:
     @overload
     def virtualenv(self, python_callable: Callable[FParams, FReturn]) -> Task[FParams, FReturn]: ...
     @overload
+    def external_python(
+        self,
+        *,
+        python_fspath: str = None,
+        multiple_outputs: Optional[bool] = None,
+        # 'python_callable', 'op_args' and 'op_kwargs' since they are filled by
+        # _PythonVirtualenvDecoratedOperator.
+        use_dill: bool = False,
+        templates_dict: Optional[Mapping[str, Any]] = None,
+        show_return_value_in_logs: bool = True,
+        **kwargs,
+    ) -> TaskDecorator:
+        """Create a decorator to convert the decorated callable to a virtual environment task.
+
+        :param python_fspath: Full time path string (file-system specific) that points to a Python binary inside
+            a virtualenv that should be used (in ``VENV/bin`` folder). Should be absolute path
+            (so usually start with "/" or "X:/" depending on the filesystem/os used).
+        :param multiple_outputs: If set, function return value will be unrolled to multiple XCom values.
+            Dict will unroll to XCom values with keys as XCom keys. Defaults to False.
+        :param use_dill: Whether to use dill to serialize
+            the args and result (pickle is default). This allow more complex types
+            but requires you to include dill in your requirements.
+        :param templates_dict: a dictionary where the values are templates that
+            will get templated by the Airflow engine sometime between
+            ``__init__`` and ``execute`` takes place and are made available
+            in your callable's context after the template has been applied.
+        :param show_return_value_in_logs: a bool value whether to show return_value
+            logs. Defaults to True, which allows return value log output.
+            It can be set to False to prevent log output of return value when you return huge data
+            such as transmission a large amount of XCom to TaskAPI.
+        """
+    @overload
+    def external_python(self, python_callable: Callable[FParams, FReturn]) -> Task[FParams, FReturn]: ...

Review Comment:
   And this can be removed if the Python executable argument can’t be None.



##########
airflow/decorators/base.py:
##########
@@ -165,7 +166,7 @@ def __init__(
         python_callable: Callable,
         task_id: str,
         op_args: Optional[Collection[Any]] = None,
-        op_kwargs: Optional[Mapping[str, Any]] = None,
+        op_kwargs: Optional[MutableMapping[str, Any]] = None,

Review Comment:
   I don’t think this needs to be mutable?



##########
airflow/operators/python.py:
##########
@@ -176,7 +178,7 @@ def execute(self, context: Context) -> Any:
 
         return return_value
 
-    def determine_kwargs(self, context: Mapping[str, Any]) -> Mapping[str, Any]:
+    def determine_kwargs(self, context: MutableMapping[str, Any]) -> MutableMapping[str, Any]:

Review Comment:
   This does not need to change either.



-- 
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@airflow.apache.org

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