uranusjr commented on a change in pull request #19487:
URL: https://github.com/apache/airflow/pull/19487#discussion_r747313873
##########
File path: airflow/api_connexion/endpoints/task_instance_endpoint.py
##########
@@ -73,9 +74,22 @@ def get_task_instance(dag_id: str, dag_run_id: str, task_id:
str, session=None):
return task_instance_schema.dump(task_instance)
+def _convert_state(states):
+ if not states:
+ return
+ for idx, item in enumerate(states):
+ # we do not support 'null' maybe we should?
+ # to support null, we should add it to TaskState in
+ # openapi doc
+ if item in ['null', 'none']:
+ states[idx] = State.NONE
+ return states
+
+
def _apply_array_filter(query, key, values):
if values is not None:
- query = query.filter(key.in_(values))
+ cond = [(key == v) for v in values]
Review comment:
```suggestion
cond = ((key == v) for v in values)
```
Nit
##########
File path: airflow/api_connexion/endpoints/task_instance_endpoint.py
##########
@@ -73,9 +74,22 @@ def get_task_instance(dag_id: str, dag_run_id: str, task_id:
str, session=None):
return task_instance_schema.dump(task_instance)
+def _convert_state(states):
+ if not states:
+ return
+ for idx, item in enumerate(states):
+ # we do not support 'null' maybe we should?
+ # to support null, we should add it to TaskState in
+ # openapi doc
+ if item in ['null', 'none']:
+ states[idx] = State.NONE
+ return states
Review comment:
Instead of modifying the list in place, would it be better to do this
instead?
```suggestion
def _convert_state(states):
if not states:
return None
return [State.NONE if item in {"null, "none"} else s for s in states]
```
Also it’d be nice to have type annotation on this function.
##########
File path: airflow/api_connexion/endpoints/task_instance_endpoint.py
##########
@@ -180,6 +197,8 @@ def get_task_instances_batch(session=None):
data = task_instance_batch_form.load(body)
except ValidationError as err:
raise BadRequest(detail=str(err.messages))
+ state = data['state']
+ states = _convert_state(state)
Review comment:
```suggestion
states = _convert_state(data['state'])
```
Let’s avoid the `state` local variable to avoid using the wrong name
accidentally.
--
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]
