potiuk commented on PR #35252: URL: https://github.com/apache/airflow/pull/35252#issuecomment-1784257754
> Sorry, I was off during the day and just now this PR jumped into my eyes. Reading the fix description and looking through the code I fear we have a more severe problem that we urgently need to fix. Termination log is just "one" thin but I now just realized that all venv script input, output, logs and temp files are also created in the venv cache folder. This is bad and a problem because multiple different functions in parallel might use the cache. So we get into a real concurrency issue! Yes. When I looked at this todya, I **thought** the lock we have is for the whole execution, but yeah. I just checked, we do it only for venv creation time. Which makes much more time. > So I propose (to have a few minutes for me) and revert this PR and change the location where the temporary script, input, output and termination log is generated in general. Every call should have its own temp for the volatile script data and we need to prevent having (static) files as temp files in the venv. I don't think there is a need or desire to revert this one - it's still useful to at least get the new test harness working https://github.com/apache/airflow/pull/35160. Reverting it now makes no sense - it at least solves "some" edge case for now. I'd say we need a new PR to fix this indeed. The scripts and output shoudl be all generated in a tmp direcrtory -- 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
