search

OpenJobDescription / openjd-sessions-for-python

Provides a Python library that can be used to build a runtime that is able to run Jobs in a Session as defined by Open Job Description.
https://github.com/OpenJobDescription/openjd-specifications/wiki
Apache License 2.0
10 stars 12 forks source link

fix: Notify should use the non-service python. #171

Closed erico-aws closed 3 weeks ago

erico-aws commented 3 weeks ago

What was the problem/requirement? (What/Why)

Notifying the windows subprocess of the CTRL_BREAK_EVENT will use the service python if running in the context of a service. It should be using the system python.

What was the solution? (How)

Replace the "pythonservice.exe" path with "python.exe"

What is the impact of this change?

Allow the CTRL_BREAK_EVENT to be sent to the subprocess correctly, allowing it to gracefully shutdown.

How was this change tested?

Ran unit tests.

Was this change documented?

Just a small comment.

Is this a breaking change?

No

Does this change impact security?

No

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

sonarcloud[bot] commented 3 weeks ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

crowecawcaw commented 3 weeks ago

The "how was this tested" info says unit tests, but because the same unit tests pass before and after this change, they don't cover it. Were you able to do some sort of manual testing to verify this doesn't break anything and that it correctly sends the signal? Also will some sort of automated testing be added? Could be a integration test that comes later.