Handle left-over termination logs from previous runs with same cache

[potiuk]

When PythonVirtualEnv and friends use cached venv, script and termination logs are stored in the venv. While this was fine for scripts because the env is locked and scripts were overwritten every time venv started - it was not the same for termination log.

The termination log remains in venv after venv completes (which is good for diagnostics) however when subsequent task failed without raising regular exception but just by sys.exit(), the termination log from previous task was found and such task would return AirflowException instead of CalledProcessError with the message coming from the previous task.

This PR fixes it by deleting the termination log at the start of PythonVirtualenv task, when such termination log is present in the venv.

---

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

[potiuk]

(Real error this time) It was found when I optimized tests by using caching. cc: @jens-scheffler-bosch :).. Very interesting side-effect - I think you should check in your implementation in Bosch if you have similar issue.

[jscheffl]

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!

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.

[potiuk]

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

[jscheffl]

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.

That concurrency thing was one important detail. The lock is only being made during creation of the venv. Once the cached venv is available of course we want and need to be able to execute tasks in the venv concurrently. Same like with external python.
But reading comments in #35258 hope it is okay to revert the termination log fix.

[potiuk]

But reading comments in #35258 hope it is okay to revert the termination log fix.

Absolutely. As if "replace with generated dir which makes it unnecessary" rather than "revert the commit :D"