[GitHub] [airflow] Jorricks edited a comment on pull request #18467: Implement test to detect functions that become a security risk because they are missing a decorator.

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

Jorricks edited a comment on pull request #18467:
URL: https://github.com/apache/airflow/pull/18467#issuecomment-925865901


   > This is a good idea, but two changes please:
   > 
   > 1. This shouldn't be in test_views_decorators.py (which is about testing the decorators themselves), but in test_views.py or one of the other files in the same folder.
   > 2. I think we can do it without inspect:
   > 
   > ```python
   > def assert_decorator_used(fn, decorator):
   >     code = decorator(None).__code__
   >     name = fn.__name__
   >     while fn is not None:
   >         if fn.__code__ is code:
   >             return
   >         fn = getattr(fn, '__wrapped__')
   >     assert False, f'{name} was not decorated with {decorator}'
   > ```
   > 
   > which is then used via `assert_decorator_used(TaskInstanceModelView.action_clear, action_has_dag_edit_access))` (for example)
   
   Good point. I agree moving it into `test_views_decorators.py`  did not makes sense. `test_views.py` seems better.
   
   I didn't know about the functionality @wraps provided. 
   Awesome. Thanks for sharing :v:, will make the changes later today.
   


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