dabla commented on PR #59876: URL: https://github.com/apache/airflow/pull/59876#issuecomment-3897670150
> Much as @kaxil said in his very first comment, I'm still -1 on this, You've changed one kind of global for another. > > before the global was `SUPERVISOR_COMMS` on the `task_runner` module. > > ```python > task_runner.SUPERVISOR_COMMS = comms_decoder > ``` > > After the global is `SupervisorComms._instance`. > > ```python > SupervisorComms().set_comms(comms_decoder) > ``` > > This is a change for no difference to my mind. Honestly I don't see the point of the change. I'm not that opinionated on that point, so I'm neutral here but I like the other refactorings that have been done: - lots of duplicated code lines has been replaced with a re-useable method named `is_client_process_context` - even more typing was applied which is always good So I think this PR has added value, but that's my personal opinion. -- 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]
