Jasperora commented on PR #52839:
URL: https://github.com/apache/airflow/pull/52839#issuecomment-3047588386
Sure! Adding type annotation for all params in methods of `BulkStateFetcher`
would make it more complete.
But in my current editing, some variables would be checked multiple times.
For example,
```python
task_results_by_task_id: dict[str, dict[str, Any]] = {
task_result["task_id"]: task_result for task_result in
task_results
}
```
ensures `task_results_by_task_id`'s type.
```python
@staticmethod
def _prepare_state_and_info_by_task_dict(
task_ids, task_results_by_task_id
task_ids, task_results_by_task_id: dict[str, dict[str, Any]]
) -> Mapping[str, EventBufferValueType]:
```
And its type is checked again when it is used as a parameter.
Is it ok? I think it might cause redundancy and inefficiency. If it is
redundant, which type annotation should I remove?
Besides, in current branch.
https://github.com/apache/airflow/blob/main/providers/celery/src/airflow/providers/celery/executors/celery_executor_utils.py#L322C4-L322C47
`sync_parallelism` is optional.
But I checked its usage in celery_executor.py.
https://github.com/apache/airflow/blob/main/providers/celery/src/airflow/providers/celery/executors/celery_executor.py#L293C5-L307C90
and
https://github.com/apache/airflow/blob/main/providers/celery/src/airflow/providers/celery/executors/celery_executor.py#L312C4-L318C73
I think `sync_parallelism` should be int instead of optional.
However, adding type annotation to it would cause error since in
`providers/celery/tests/integration/celery/test_celery_executor.py` some
`BulkStateFetcher` are not provided with that parameter.
For example,
https://github.com/apache/airflow/blob/main/providers/celery/tests/integration/celery/test_celery_executor.py#L331C17-L331C67
Do I need to give a fake value to `BulkStateFetcher` in the test?
--
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]
