[GitHub] [airflow] hussein-awala commented on pull request #32627: Replace State by TaskInstanceState in Airflow executors

["hussein-awala (via GitHub)" <gi...@apache.org>]

hussein-awala commented on PR #32627:
URL: https://github.com/apache/airflow/pull/32627#issuecomment-1668688810

   @uranusjr could you check it now?
   
   I'm not sure about this change in base executor:
   ```diff
   -    def change_state(self, key: TaskInstanceKey, state: str, info=None) -> None:
   +    def change_state(self, key: TaskInstanceKey, state: TaskInstanceState, info=None) -> None:
   ```
   Is it a breaking change? If so, I can change the signature in the base executor (only base executor) to make it `TaskInstanceState | str`, and convert it to `TaskInstanceState` if it's str. WDYT?
   
   > The executor interface itself (the BaseExecutor class) is public, but the built-in executors are not (i.e. KubernetesExecutor, LocalExecutor, etc).
   
   


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