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 2021/12/01 05:03:39 UTC

[GitHub] [airflow] yiga2 opened a new issue #19922: ShortCircuitOperator does not save to xcom

yiga2 opened a new issue #19922:
URL: https://github.com/apache/airflow/issues/19922


   ### Description
   
   `ShortCircuitOperator` does not return any value regardless of the result. 
   
   Still, if `condition` evaluates to `falsey`, it can be useful to store/save the result of the condition to XCom so that downstream tasks. can use. 
   Many objects evaluates to True/False, not just booleans - see  https://docs.python.org/3/library/stdtypes.html
   
   ### Use case/motivation
   
   The change is trivial and would allow to combine a Python task and ShortCircuit into one.:
   
   1. if callable returns None (or False) -> skip
   2. if callable returns non-empty object (or True) -> continue. The proposed change is to pass on the condition to XCom.
   
   ### Related issues
   
   _No response_
   
   ### Are you willing to submit a PR?
   
   - [X] Yes I am willing to submit a PR!
   
   ### Code of Conduct
   
   - [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
   


-- 
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] yiga2 commented on issue #19922: ShortCircuitOperator does not save to xcom

Posted by GitBox <gi...@apache.org>.
yiga2 commented on issue #19922:
URL: https://github.com/apache/airflow/issues/19922#issuecomment-983299360


   We currently check if file(s) exist in a folder (e.g. GCS) and if not empty, parse the latest file and extract 3 data elements that will trigger a whole branch. 
   If nothing, we skip all tasks in that branch up until it merges back to the "main stream".
   


-- 
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] eladkal edited a comment on issue #19922: ShortCircuitOperator does not save to xcom

Posted by GitBox <gi...@apache.org>.
eladkal edited a comment on issue #19922:
URL: https://github.com/apache/airflow/issues/19922#issuecomment-983300641


   > We currently check if file(s) exist in a folder (e.g. GCS) and if not empty, parse the latest file and extract 3 data elements that will trigger a whole branch. If nothing, we skip all tasks in that branch up until it merges back to the "main stream".
   
   So why do you need the True/False value of ShortCircuitOperator in Xcom?


-- 
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] yiga2 commented on issue #19922: ShortCircuitOperator does not save to xcom

Posted by GitBox <gi...@apache.org>.
yiga2 commented on issue #19922:
URL: https://github.com/apache/airflow/issues/19922#issuecomment-983302641


   There is no True/False to save. The `callable` returns `None` or a `dict`, and I want to have the `dict` saved.
   The code evaluates the condition like so:
   `if condition:` and it doesn't force for it to be a boolean - which is great.


-- 
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] eladkal edited a comment on issue #19922: ShortCircuitOperator does not save to xcom

Posted by GitBox <gi...@apache.org>.
eladkal edited a comment on issue #19922:
URL: https://github.com/apache/airflow/issues/19922#issuecomment-983294920


   Can you describe a usecase where this is usefull?
   I'm curious because currently the operator skip all downstream tasks so if you have a usecase where something like that is needed it might not work as you expect? ( See https://github.com/apache/airflow/issues/7858 )


-- 
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] yiga2 commented on issue #19922: ShortCircuitOperator does not save to xcom

Posted by GitBox <gi...@apache.org>.
yiga2 commented on issue #19922:
URL: https://github.com/apache/airflow/issues/19922#issuecomment-989036255


   @josh-fell just to clarify that a use case - not mine - may not involve the use of a `BranchPythonOperator` right (or any time) after, so no real dependency or downside. Any downstream task could just use the result of the `ShortCircuitOperator` evaluation with the change, whereas today it can't.


-- 
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] eladkal commented on issue #19922: ShortCircuitOperator does not save to xcom

Posted by GitBox <gi...@apache.org>.
eladkal commented on issue #19922:
URL: https://github.com/apache/airflow/issues/19922#issuecomment-983300641


   > We currently check if file(s) exist in a folder (e.g. GCS) and if not empty, parse the latest file and extract 3 data elements that will trigger a whole branch. If nothing, we skip all tasks in that branch up until it merges back to the "main stream".
   
   So why do you need the True/False value of ShortCircuitOperator?


-- 
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 issue #19922: ShortCircuitOperator does not save to xcom

Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #19922:
URL: https://github.com/apache/airflow/issues/19922#issuecomment-985211860


   +1 on seeing a simple or analogous example. 
   
   No reason why the `callable` used in the `ShortCircuitOperator` couldn't push an `XCom` and then a downstream `BranchPythonOperator` uses that `XCom` to determine which branch(es) to follow. Of course, you're still up against #7858 being implemented first like you mentioned.


-- 
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] eladkal commented on issue #19922: ShortCircuitOperator does not save to xcom

Posted by GitBox <gi...@apache.org>.
eladkal commented on issue #19922:
URL: https://github.com/apache/airflow/issues/19922#issuecomment-983294920


   Can you describe a usecase where this is usefull?
   I'm curious because currently the operator skip all downstream tasks so if you have a usecase where something like that is needed it might not work as you expect?


-- 
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] yiga2 edited a comment on issue #19922: ShortCircuitOperator does not save to xcom

Posted by GitBox <gi...@apache.org>.
yiga2 edited a comment on issue #19922:
URL: https://github.com/apache/airflow/issues/19922#issuecomment-983302641


   There is no True/False to save. The `callable` returns in our case `None` or a `dict`, and I want to have the `dict` saved.
   
   The code evaluates the condition like so:
   `if condition:` 
   and it doesn't force for it to be a boolean - which is great - so the change would be trivial:
   
   [return](https://github.com/apache/airflow/blob/3d9e4558cfff7f8a9383d1501364c97b7df483da/airflow/operators/python.py#L250)-> `return condition`
   
   
   However, I read that  #7858 is also needed for our use case if we want to get back to standard airflow. Maybe this request can be merged with the PR.


-- 
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] eladkal closed issue #19922: ShortCircuitOperator does not save to xcom

Posted by GitBox <gi...@apache.org>.
eladkal closed issue #19922:
URL: https://github.com/apache/airflow/issues/19922


   


-- 
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] eladkal closed issue #19922: ShortCircuitOperator does not save to xcom

Posted by GitBox <gi...@apache.org>.
eladkal closed issue #19922:
URL: https://github.com/apache/airflow/issues/19922


   


-- 
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] yiga2 edited a comment on issue #19922: ShortCircuitOperator does not save to xcom

Posted by GitBox <gi...@apache.org>.
yiga2 edited a comment on issue #19922:
URL: https://github.com/apache/airflow/issues/19922#issuecomment-983302641


   There is no True/False to save. The `callable` returns `None` or a `dict`, and I want to have the `dict` saved.
   The code evaluates the condition like so:
   `if condition:` and it doesn't force for it to be a boolean - which is great.
   However, I read that  #7858 is also needed for our use case if we want to get back to standard airflow. Maybe this request can be merged with the PR.


-- 
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] eladkal commented on issue #19922: ShortCircuitOperator does not save to xcom

Posted by GitBox <gi...@apache.org>.
eladkal commented on issue #19922:
URL: https://github.com/apache/airflow/issues/19922#issuecomment-983317326


   The feature request is to push to xcom - your explnation doesnt say anything about what pulls from xcom. If you can show a simple dag that uses the feature it will be easier to understand.
   
   Regardless - that is just me being curious. You can ofcourse submit a PR and we will review it.


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