Skip to content

Conversation

@jblagden
Copy link
Contributor

Fixes: #113

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

When the adaptor fails to find the set executable, it'll fail with [WinError 2] The system cannot find the file specified, which without any context doesn't set users up to sort the issue out.

What was the solution? (How)

This change adds user friendly messages about what's failing and possible reasons why.

What is the impact of this change?

Users will see more logging when the adaptor fails to find the executable it has been set to run.

How was this change tested?

Currently a gap in this PR - I'm not sure where to start testing if a file does or does not exist and modifying the environment within a test.

See DEVELOPMENT.md for information on running tests.

  • Have you run the unit tests?

Was this change documented?

Yes, to failures on TestIntegrationClientInterface, TestIntegrationLoggingSubprocess, and TestDaemonMode:

FAILED test/openjd/adaptor_runtime_client/integ/test_integration_client_interface.py::TestIntegrationClientInterface::test_graceful_shutdown - AssertionError: assert 'Received SIGTERM signal.' in ''
FAILED test/openjd/adaptor_runtime/integ/process/test_integration_logging_subprocess.py::TestIntegrationLoggingSubprocess::test_stop_process[StopProcessWhenSIGTERMFails] - Failed: Timeout >5.0s
FAILED test/openjd/adaptor_runtime/integ/background/test_background_mode.py::TestDaemonMode::test_shutdown - psutil.TimeoutExpired: timeout after 1 seconds (pid=11312)

I'm working in the Windows subsystem for Linux which might be the cause of the failing timeouts but I'm not sure about the SIGTERM issue. I'm hoping the github runners shed a little light on what's happening.

  • Are relevant docstrings in the code base updated?

Yes.

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.

@jblagden jblagden requested a review from a team as a code owner August 22, 2025 13:34
Comment on lines +82 to +85
if exe_path is not None:
raise FileNotFoundError(
f"Could not find executable at: {exe_path} using alias {executable}\n"
f"Error:{fnf_error}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my own curiosity, how does a user end up in this situation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If an alias gets broken this'll happen. I can't imagine this really ever gets run, but if it does we'd be happy to have it. :)

…tor's executable on the system PATH.

Signed-off-by: Justin Blagden <jblagden@users.noreply.github.com>
…sage.

Signed-off-by: Justin Blagden <jblagden@users.noreply.github.com>
Signed-off-by: Justin Blagden <jblagden@users.noreply.github.com>
@epmog epmog force-pushed the improve_subprocess_exectuable_message branch from e850f81 to 3d78c02 Compare September 16, 2025 20:44
@epmog epmog enabled auto-merge (squash) September 16, 2025 20:44
@sonarqubecloud
Copy link

@epmog epmog merged commit e908f47 into OpenJobDescription:mainline Sep 16, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: More informative error messages when a subprocess executable cannot be found.

4 participants