don't kill if pid same as file (#8997) (#8998)

[lewijw]

The pid file needs to be deleted.

Note: Before submitting this pull request, please review our contributing
guidelines.

Description

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.81%. Comparing base (a68f3aa) to head (ebac57d).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9007   +/-   ##
=======================================
  Coverage   77.81%   77.81%           
=======================================
  Files         150      150           
  Lines       18688    18689    +1     
  Branches     3194     3194           
=======================================
+ Hits        14542    14543    +1     
  Misses       3854     3854           
  Partials      292      292           

Flag	Coverage Δ
unittests	77.79% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[auvipy]

can you please better explain the new change? do you think an integration test is possible to add for this part?

[Nusnus]

@lewijw Hey there,

I’m a bit confused - wasn’t there another PR about this? #8998
If it has a bug, I’d prefer to revert it and then create a new PR for the fully fixed and tested changes, unless this can be solved ASAP (e.g., answering @auvipy).

I’m trying to make sure main is g2g 99.9%, so I’d appreciate a bit of clarification.

Thank you.

[lewijw]

@auvipy and @Nusnus, I understand your concern, and I am not sure why I did not see this issue earlier because as soon as remove_if_stale returns, the acquire() function is called which tries to write the pid file. So, why I am just now getting a "file exists" failure is a bit confusing to me. However, I do believe that a revert would not bring the codebase to a better state because the code kills the process that it is running in. I am also very certain that there will be no changes after this commit.
As far as creating an integrated test, that would be very difficult because you have to make sure the process id remains the same between runs, and as far as I know, the only way to make that happen is to run the process in a k8s pod.

[lewijw]

Note also that every time remove_if_stale returns True, it removes the pid file first. I was reluctant to do the same at first until I was certain it was required. Now it looks like it is required.