TJaniF commented on PR #29842:
URL: https://github.com/apache/airflow/pull/29842#issuecomment-1471013878

   > > Users who are using the GitHubSensor directly and pass a 
`result_processor` function directly would either need to turn 
`allow_templates_in_result_processor` to False or add a `templated_fields` 
kwarg to their result_processor function. That is probably affecting more users 
than custom operators and it seems like it could be potentially confusing.
   > 
   > Only if they use inputs that look like templates but actually are not, I 
think? Because otherwise the input can still be template-rendered and would not 
change anyway. (It might be ever so slightly slower but shouldn’t break.) That 
feels like a very specific edge case to me.
   
   I'm sorry I think I explained this poorly. 😅 
   I think the issue is that when `allow_templates_in_result_processor=True` in 
the GithubSensor, the function passed to result_processor is expected to have a 
parameter called `templated_fields`. 
   
   With the first version of this PR (commit 
[fe97f18](https://github.com/apache/airflow/pull/29842/commits/fe97f1808a557b195dd341c8fefb5d87e82d9af3)
 the following task fails:
   
   ```python
   def max_10_branches(repo):
           result = None 
           try:
               if repo is not None:
                   branches = repo.get_branches()
                   if branches.totalCount <= 10:
                       result = True
                   else: 
                       result = False
           except Exception as e:
               raise AirflowException(f"Github sensor error: {str(e)}")
           return result
   
       using_first_version_githubsensor = FIRST_VERSION_GithubSensor(
           task_id="first_version_sensor",
           github_conn_id='github_default',
           method_name="get_repo",
           method_params={'full_name_or_id': GITHUB_REPOSITORY},
           result_processor=lambda repo: max_10_branches(repo),
           allow_templates_in_result_processor=True
       )
   ```
   
   with
   
   ```text
   [2023-03-15, 23:57:30 UTC] {taskinstance.py:1768} ERROR - Task failed with 
exception
   Traceback (most recent call last):
     File "/usr/local/lib/python3.9/site-packages/airflow/sensors/base.py", 
line 199, in execute
       poke_return = self.poke(context)
     File "/usr/local/airflow/include/custom_githubtagsensor_old.py", line 68, 
in poke
       return self.result_processor(github_result, 
templated_fields=templated_fields)
   TypeError: <lambda>() got an unexpected keyword argument 'templated_fields'
   ```
   
   this is why I think setting `allow_templates_in_result_processor=True` in 
the GithubSensor could lead to an odd pattern of having to include this kwarg 
in any function passed to `result_processor`.
   
   The current version of the PR prevents this from happening and means the 
GithubSensor can be used exactly the same as it is right now, the only change 
is that operators inheriting from it can now use templated args in their 
processor function if the parameter `templated_processor` is used instead of 
`result_processor`. As far as I can tell we get the same functionality without 
any breakage.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to