potiuk commented on issue #37484: URL: https://github.com/apache/airflow/issues/37484#issuecomment-1958442204
Just a comment. I am not sure if that is a problem. If you look at where the `_airflow_parsing_context_manager` is used, it is used, right before running subproces.Popen or right after the process has been forked and `setproctitle` called on it - which in fact means, that yeah, we are 100% sure we have no other threads running at this time. And we actualy only start Threads after that (so actually even outside of the context manager): https://github.com/apache/airflow/blob/main/airflow/task/task_runner/base_task_runner.py#L134 https://github.com/apache/airflow/blob/main/airflow/providers/celery/executors/celery_executor_utils.py#L130 https://github.com/apache/airflow/blob/main/airflow/task/task_runner/standard_task_runner.py#L101 And the env variables are used in this case because the environment variables are then passed to subprocesses without modifying the process parameters (if the child process is started by spawning a new interpreter) and automatically available to forked processes when start_by_fork is used. none of the "spawn/fork" is affected by thread-safety (those new processes, not threads) so there is no problem with thread-safety, Basically at the moment the context manager is used there is only single main thread running in the process, so IMHO yes - we are 100% sure that no other threads have been started yet. And benefit of using env variables in this case is that passing and reading them works the same - regardless if the child process is spawned via Popen in a new interpreter or whether it's forked. Plain Global variable **could** work for forks - because global variables are available in forks, but if you want to impleement replsacement, you will have to also handle passing the context to subprocesses spawned by Popen - for exmple via extra arguments passed or (surprise) via env variable. You can probably figure two different mechanisms to pass and read the values for those two cases, but IMHO it's not needed. WDYT @hterik ? How did you want to implement passing of the context to spawned subproceses? Global Python variable is not nearly good enough for that). -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
