potiuk commented on code in PR #34317:
URL: https://github.com/apache/airflow/pull/34317#discussion_r1355894015
##########
airflow/api_connexion/endpoints/task_instance_endpoint.py:
##########
@@ -61,13 +61,8 @@
T = TypeVar("T")
[email protected]_access(
- [
- (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
- (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN),
- (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
- ],
-)
[email protected]_access_dag("GET", DagAccessEntity.RUN)
[email protected]_access_dag("GET", DagAccessEntity.TASK_INSTANCE)
Review Comment:
Finally got to review it after the break (and this and next week will have a
lot of focus on those).
I think this really calls for small refactoring:
```python
def requires_access_dag(
method: ResourceMethod, access_entities: tuple[DAGAccessEntity] |
DagAccessEntity | None = None
```
and then:
```python
@security.requires_access_dag("GET", (DagAccessEntity.RUN,
DagAccessEntity.TASK_INSTANCE))
```
and convert `access_entity` to `access_entities: tuple[DAGDetails]` (we can
convert single dag detail to one-element tuple).
Not sure if now or as a follow up to this PR, but I am quite sure this will
be much better for performance point of view (one callback instead of two and
it will be pretty much impossible to optimize it if we have two decorators.
And since it will change the callback, it's likely better to do it it now.
--
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]
