You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "dabla (via GitHub)" <gi...@apache.org> on 2026/10/05 06:51:29 UTC

[PR] Allow MSGraphAsyncOperator and MSGraphSensor to directly start from trigger instead of worker [airflow]

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

   <!--
    Licensed to the Apache Software Foundation (ASF) under one
    or more contributor license agreements.  See the NOTICE file
    distributed with this work for additional information
    regarding copyright ownership.  The ASF licenses this file
    to you under the Apache License, Version 2.0 (the
    "License"); you may not use this file except in compliance
    with the License.  You may obtain a copy of the License at
   
      http://www.apache.org/licenses/LICENSE-2.0
   
    Unless required by applicable law or agreed to in writing,
    software distributed under the License is distributed on an
    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    KIND, either express or implied.  See the License for the
    specific language governing permissions and limitations
    under the License.
    -->
   
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of an existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   Allow MSGraphAsyncOperator and MSGraphSensor to always start from trigger instead of worker as this avoids unnecessary round trips to the scheduler.
   
   <!-- Please keep an empty line above the dashes. -->
   ---
   **^ Add meaningful description above**
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
   In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
   


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


Re: [PR] Allow MSGraphAsyncOperator and MSGraphSensor to directly start from trigger instead of worker [airflow]

Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #53009:
URL: https://github.com/apache/airflow/pull/53009#discussion_r2192165544


##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/operators/msgraph.py:
##########
@@ -108,8 +125,17 @@ class MSGraphAsyncOperator(BaseOperator):
         the message from the event, otherwise the response from the event payload is returned.
     :param serializer: Class which handles response serialization (default is ResponseSerializer).
         Bytes will be base64 encoded into a string, so it can be stored as an XCom.
+    :param start_from_trigger: If set to True, the operator will start directly from the triggerer without going into the worker first.
     """
 
+    start_trigger_args = StartTriggerArgs(
+        trigger_cls=f"{MSGraphTrigger.__module__}.{MSGraphTrigger.__name__}",
+        trigger_kwargs={},
+        next_method="execute_complete",
+        next_kwargs=None,
+        timeout=None,
+    )
+    start_from_trigger = False

Review Comment:
   Is there a reason you we wouldn't want this to be true by default? Given all the execute does is defer, I don't even think this needs to be an option -- just set it to true always? And then `execute()` is only called for when running on older versions of Airflow



##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/operators/msgraph.py:
##########
@@ -35,6 +37,21 @@
 from airflow.providers.microsoft.azure.version_compat import BaseOperator
 from airflow.utils.xcom import XCOM_RETURN_KEY
 
+try:
+    from airflow.triggers.base import StartTriggerArgs
+except ImportError:
+    # TODO: Remove this when min airflow version is 2.10.0 for standard provider

Review Comment:
   ```suggestion
       # TODO: Remove this when min airflow version is 2.10.0
   ```



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


Re: [PR] Allow MSGraphAsyncOperator and MSGraphSensor to directly start from trigger instead of worker [airflow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #53009: Allow MSGraphAsyncOperator and MSGraphSensor to directly start from trigger instead of worker
URL: https://github.com/apache/airflow/pull/53009


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


Re: [PR] Allow MSGraphAsyncOperator and MSGraphSensor to directly start from trigger instead of worker [airflow]

Posted by "dabla (via GitHub)" <gi...@apache.org>.
dabla commented on PR #53009:
URL: https://github.com/apache/airflow/pull/53009#issuecomment-3051428503

   This [PR ](https://github.com/apache/airflow/pull/53071) must be merged before this one can.


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


Re: [PR] Allow MSGraphAsyncOperator and MSGraphSensor to directly start from trigger instead of worker [airflow]

Posted by "dabla (via GitHub)" <gi...@apache.org>.
dabla commented on code in PR #53009:
URL: https://github.com/apache/airflow/pull/53009#discussion_r2192610832


##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/operators/msgraph.py:
##########
@@ -108,8 +125,17 @@ class MSGraphAsyncOperator(BaseOperator):
         the message from the event, otherwise the response from the event payload is returned.
     :param serializer: Class which handles response serialization (default is ResponseSerializer).
         Bytes will be base64 encoded into a string, so it can be stored as an XCom.
+    :param start_from_trigger: If set to True, the operator will start directly from the triggerer without going into the worker first.
     """
 
+    start_trigger_args = StartTriggerArgs(
+        trigger_cls=f"{MSGraphTrigger.__module__}.{MSGraphTrigger.__name__}",
+        trigger_kwargs={},
+        next_method="execute_complete",
+        next_kwargs=None,
+        timeout=None,
+    )
+    start_from_trigger = False

Review Comment:
   I've refactored code to always run from triggerer and modified tests to support this



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


Re: [PR] Allow MSGraphAsyncOperator and MSGraphSensor to directly start from trigger instead of worker [airflow]

Posted by "dabla (via GitHub)" <gi...@apache.org>.
dabla commented on PR #53009:
URL: https://github.com/apache/airflow/pull/53009#issuecomment-3049859828

   Ok overriding the expand_start_trigger_args doesn't help because the scheduler has a serialized version of the task (e.g. operator), so it won't take into account that overriden method.  So now remains the question, how do we fix this issue.  This is clearly a situation which hasn't been thought about.


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


Re: [PR] Allow MSGraphAsyncOperator and MSGraphSensor to directly start from trigger instead of worker [airflow]

Posted by "dabla (via GitHub)" <gi...@apache.org>.
dabla commented on code in PR #53009:
URL: https://github.com/apache/airflow/pull/53009#discussion_r2192489101


##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/operators/msgraph.py:
##########
@@ -108,8 +125,17 @@ class MSGraphAsyncOperator(BaseOperator):
         the message from the event, otherwise the response from the event payload is returned.
     :param serializer: Class which handles response serialization (default is ResponseSerializer).
         Bytes will be base64 encoded into a string, so it can be stored as an XCom.
+    :param start_from_trigger: If set to True, the operator will start directly from the triggerer without going into the worker first.
     """
 
+    start_trigger_args = StartTriggerArgs(
+        trigger_cls=f"{MSGraphTrigger.__module__}.{MSGraphTrigger.__name__}",
+        trigger_kwargs={},
+        next_method="execute_complete",
+        next_kwargs=None,
+        timeout=None,
+    )
+    start_from_trigger = False

Review Comment:
   Yes indeed, fully agree, but I did this to allow the tests to run, which  is a bad reason but would need some rework



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


Re: [PR] Allow MSGraphAsyncOperator and MSGraphSensor to directly start from trigger instead of worker [airflow]

Posted by "dabla (via GitHub)" <gi...@apache.org>.
dabla commented on PR #53009:
URL: https://github.com/apache/airflow/pull/53009#issuecomment-3049784739

   I think there is an issue with the defer_task method of TaskInstance, it should call render_template_fields first before calling the [expand_start_trigger_args ](https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/models/taskinstance.py#L1607) method.


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


Re: [PR] Let MSGraphAsyncOperator and MSGraphSensor start from the triggerer [airflow]

Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on PR #53009:
URL: https://github.com/apache/airflow/pull/53009#issuecomment-6489842769

   @dabla Could you please resolve conflicts?


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


Re: [PR] Allow MSGraphAsyncOperator and MSGraphSensor to directly start from trigger instead of worker [airflow]

Posted by "dabla (via GitHub)" <gi...@apache.org>.
dabla commented on PR #53009:
URL: https://github.com/apache/airflow/pull/53009#issuecomment-3246843750

   Created new [PR](https://github.com/apache/airflow/pull/55068) which attempts to fix template rendering with start from trigger, as long as template rendering in triggerer isn't fixed, this PR can't be merged.


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


Re: [PR] Allow MSGraphAsyncOperator and MSGraphSensor to directly start from trigger instead of worker [airflow]

Posted by "dabla (via GitHub)" <gi...@apache.org>.
dabla commented on code in PR #53009:
URL: https://github.com/apache/airflow/pull/53009#discussion_r2192525827


##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/operators/msgraph.py:
##########
@@ -108,8 +125,17 @@ class MSGraphAsyncOperator(BaseOperator):
         the message from the event, otherwise the response from the event payload is returned.
     :param serializer: Class which handles response serialization (default is ResponseSerializer).
         Bytes will be base64 encoded into a string, so it can be stored as an XCom.
+    :param start_from_trigger: If set to True, the operator will start directly from the triggerer without going into the worker first.
     """
 
+    start_trigger_args = StartTriggerArgs(
+        trigger_cls=f"{MSGraphTrigger.__module__}.{MSGraphTrigger.__name__}",
+        trigger_kwargs={},
+        next_method="execute_complete",
+        next_kwargs=None,
+        timeout=None,
+    )
+    start_from_trigger = False

Review Comment:
   I'll try to refactor the tests to support this aspect only, it would then indeed make more sense to always start the operator from the triggerer



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


Re: [PR] Allow MSGraphAsyncOperator and MSGraphSensor to directly start from trigger instead of worker [airflow]

Posted by "dabla (via GitHub)" <gi...@apache.org>.
dabla commented on code in PR #53009:
URL: https://github.com/apache/airflow/pull/53009#discussion_r2192491425


##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/operators/msgraph.py:
##########
@@ -35,6 +37,21 @@
 from airflow.providers.microsoft.azure.version_compat import BaseOperator
 from airflow.utils.xcom import XCOM_RETURN_KEY
 
+try:
+    from airflow.triggers.base import StartTriggerArgs
+except ImportError:
+    # TODO: Remove this when min airflow version is 2.10.0 for standard provider

Review Comment:
   copy/pasta 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@airflow.apache.org

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


Re: [PR] Allow MSGraphAsyncOperator and MSGraphSensor to directly start from trigger instead of worker [airflow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #53009:
URL: https://github.com/apache/airflow/pull/53009#issuecomment-4043055199

   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.


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