potiuk commented on PR #31033: URL: https://github.com/apache/airflow/pull/31033#issuecomment-1533522668
Nice. I like it - this is a simple solution and I think It is also generally a practice we are implementing more and more - splitting modules in order to get rid unnecessary coupling - especially with introducing more type hinting, some of the implicit couplings have been made explicit - like this one. One thing to add though: https://github.com/apache/airflow/blob/main/docs/conf.py#L248 should also include taskinstancekey and it should also be added in https://github.com/apache/airflow/blob/main/docs/apache-airflow/public-airflow-interface.rst nest to taskinstance package as they are part of the public Airflow interface. I also think it is backwards-compatible since TaskInstanceKey is imported in taskinstance.py (so if someone imports it from there, it will continue to work). I would love another maintainer opinion, but it looks good to me. -- 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]
