You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/08/09 09:15:32 UTC

[GitHub] [airflow] uranusjr opened a new pull request, #25617: Promote Operator.output more

uranusjr opened a new pull request, #25617:
URL: https://github.com/apache/airflow/pull/25617

   See #25604. I feel this is a better syntax than importing XComArg?


-- 
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


[GitHub] [airflow] kaxil commented on a diff in pull request #25617: Promote Operator.output more

Posted by GitBox <gi...@apache.org>.
kaxil commented on code in PR #25617:
URL: https://github.com/apache/airflow/pull/25617#discussion_r944340137


##########
docs/apache-airflow/concepts/dynamic-task-mapping.rst:
##########
@@ -180,17 +180,19 @@ It is possible to use ``partial`` and ``expand`` with classic style operators as
 Mapping over result of classic operators
 ----------------------------------------
 
-If you want to map over the result of a classic operator you will need to create an ``XComArg`` object manually.
+If you want to map over the result of a classic operator, you should explicitly reference the *output*, instead of the operator itself.
 
 .. code-block:: python
 
-    from airflow import XComArg
+    # Create a list of data inputs.
+    extract = ExtractOperator(task_id="extract")
 
-    task = MyOperator(task_id="source")
+    # Expand the operator to transform each input.
+    transform = TransformOperator.partial(task_id="transform").expand(input=extract.output)
 
-    downstream = MyOperator2.partial(task_id="consumer").expand(input=XComArg(task))
+    # Collect the transformed inputs, expand the operator to load each one of them to the target.
+    load = LoadOperator(task_id="load").expand(input=transform.output)

Review Comment:
   `.partial` is missing, I don't think you can `expand` after you have already instantiated your operator
   ```suggestion
       load = LoadOperator.partial(task_id="load").expand(input=transform.output)
   ```



-- 
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


[GitHub] [airflow] uranusjr commented on a diff in pull request #25617: Promote Operator.output more

Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #25617:
URL: https://github.com/apache/airflow/pull/25617#discussion_r941966475


##########
docs/apache-airflow/concepts/dynamic-task-mapping.rst:
##########
@@ -180,17 +180,14 @@ It is possible to use ``partial`` and ``expand`` with classic style operators as
 Mapping over result of classic operators
 ----------------------------------------
 
-If you want to map over the result of a classic operator you will need to create an ``XComArg`` object manually.
+If you want to map over the result of a classic operator, you need to explicitly reference the *output*, instead of the operator itself.

Review Comment:
   The *need to* part is contrast to just passing in the operator object (i.e. not add `.output`), which wouldn’t work, so *can* would be confusing to me as well. Maybe *should*?



-- 
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


[GitHub] [airflow] uranusjr merged pull request #25617: Promote Operator.output more

Posted by GitBox <gi...@apache.org>.
uranusjr merged PR #25617:
URL: https://github.com/apache/airflow/pull/25617


-- 
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


[GitHub] [airflow] josh-fell commented on a diff in pull request #25617: Promote Operator.output more

Posted by GitBox <gi...@apache.org>.
josh-fell commented on code in PR #25617:
URL: https://github.com/apache/airflow/pull/25617#discussion_r941564320


##########
docs/apache-airflow/concepts/dynamic-task-mapping.rst:
##########
@@ -180,17 +180,14 @@ It is possible to use ``partial`` and ``expand`` with classic style operators as
 Mapping over result of classic operators
 ----------------------------------------
 
-If you want to map over the result of a classic operator you will need to create an ``XComArg`` object manually.
+If you want to map over the result of a classic operator, you need to explicitly reference the *output*, instead of the operator itself.

Review Comment:
   ```suggestion
   If you want to map over the result of a classic operator, you can explicitly reference the *output*, instead of the operator itself.
   ```
   Nit. Technically you _could_ still use `XComArg()` as well but we're saying there is a better way.



-- 
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


[GitHub] [airflow] uranusjr commented on pull request #25617: Promote Operator.output more

Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #25617:
URL: https://github.com/apache/airflow/pull/25617#issuecomment-1210124306

   Adding an example for mapped tasks sounds good to me. Since this PR needs #25604 to be merged first (there are failing tests that rely on `output` being available), let’s get that other PR merged first and I’ll add the example here.


-- 
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


[GitHub] [airflow] josh-fell commented on a diff in pull request #25617: Promote Operator.output more

Posted by GitBox <gi...@apache.org>.
josh-fell commented on code in PR #25617:
URL: https://github.com/apache/airflow/pull/25617#discussion_r941974385


##########
docs/apache-airflow/concepts/dynamic-task-mapping.rst:
##########
@@ -180,17 +180,14 @@ It is possible to use ``partial`` and ``expand`` with classic style operators as
 Mapping over result of classic operators
 ----------------------------------------
 
-If you want to map over the result of a classic operator you will need to create an ``XComArg`` object manually.
+If you want to map over the result of a classic operator, you need to explicitly reference the *output*, instead of the operator itself.

Review Comment:
   Ah I see. That's fair. Words are fun.



-- 
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