Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
4b110f3
Adds user-friendly messaging when the adaptor executable isn't found.
jblagden Jan 21, 2025
1fee6ec
feat! Adds user-friendly messaging when the adaptor executable isn't …
jblagden Jan 21, 2025
cefeb69
Merge branch 'better_error_subprocess_executable' of https://github.c…
jblagden Jan 27, 2025
687b384
Resolves code smells called out by sonarqube
jblagden Jan 28, 2025
61c67a1
Adds a test for executable not found error
jblagden Feb 3, 2025
85bad29
Adds a test for executable not found error
jblagden Feb 3, 2025
2540d7b
Merge branch 'better_error_subprocess_executable' of https://github.c…
jblagden Feb 3, 2025
702c8aa
Merge branch 'mainline' into better_error_subprocess_executable
jblagden Feb 3, 2025
b24d36d
Merge branch 'better_error_subprocess_executable' of https://github.c…
jblagden Feb 3, 2025
b9141bd
Merge branch 'better_error_subprocess_executable' of https://github.c…
jblagden Feb 21, 2025
8e59494
Updates test assertions to include the whole executable path
jblagden Feb 21, 2025
b938841
Merge branch 'OpenJobDescription:mainline' into better_error_subproce…
jblagden Jun 18, 2025
5ff9e53
Adds a test for executable not found error
jblagden Feb 3, 2025
4adb9cc
Merge branch 'better_error_subprocess_executable' of https://github.c…
jblagden Aug 21, 2025
3f320fc
chore(deps): update python-semantic-release requirement (#159)
dependabot[bot] Nov 28, 2024
68ad896
chore: allow pywin32 >= 307 (#162)
joel-wong-aws Nov 28, 2024
c41feab
chore(release): 0.8.2 (#165)
client-software-ci Nov 29, 2024
415ef9f
fix!: Ensure all open uses UTF-8 instead of default encoding. (#171)
karthikbekalp Dec 23, 2024
e93ef36
chore(release): 0.9.0 (#173)
client-software-ci Jan 2, 2025
a87840a
feat! Adds user-friendly messaging when the adaptor executable isn't …
jblagden Jan 21, 2025
4c89a0a
Resolves code smells called out by sonarqube
jblagden Jan 28, 2025
d800bbd
Adds a test for executable not found error
jblagden Feb 3, 2025
e34036d
chore: update GitHub issue templates (#176)
joel-wong-aws Jan 27, 2025
1984974
chore: increase log level for daemon _serve args to info (#169)
joel-wong-aws Jan 31, 2025
5dd3862
chore: add maintenance label to maintenance GitHub template (#182)
joel-wong-aws Feb 20, 2025
77104bc
chore(deps): update python-semantic-release requirement (#181)
dependabot[bot] Feb 21, 2025
1b0de05
chore(deps): update hatch requirement from ==1.13.* to ==1.14.* (#168)
dependabot[bot] Feb 21, 2025
f4d7793
chore(deps): update ruff requirement from ==0.8.* to ==0.9.* (#175)
dependabot[bot] Feb 21, 2025
ea7852a
test: Broaden regex to ignore copyright header file created by setupt…
moorec-aws Feb 26, 2025
d554197
chore(deps): update psutil requirement from ==6.1.* to ==7.0.* (#185)
dependabot[bot] Feb 26, 2025
10e9aa1
chore(deps): update black requirement from ==24.* to ==25.* (#184)
dependabot[bot] Feb 26, 2025
5ad6501
chore(deps): update python-semantic-release requirement (#183)
dependabot[bot] Feb 26, 2025
6fe1018
chore(deps): update ruff requirement from ==0.9.* to ==0.11.* (#189)
dependabot[bot] Mar 21, 2025
313d2ab
chore(deps): update pytest-cov requirement from ==6.0.* to ==6.1.* (#…
dependabot[bot] Apr 8, 2025
574ba15
chore: changes for mypy update (#191)
edwards-aws Apr 11, 2025
f9010ea
chore(deps): update pytest-timeout requirement from ==2.3.* to ==2.4.…
dependabot[bot] May 14, 2025
9fde718
chore(deps): update pytest to 8.3.* (#193)
moorec-aws May 14, 2025
26877c7
chore(deps): update hatch-vcs requirement from ==0.4.* to ==0.5.*
dependabot[bot] Jun 2, 2025
ac63a2e
chore(deps): update pytest-xdist requirement from ==3.6.* to ==3.7.*
dependabot[bot] Jun 3, 2025
4990ab3
chore(deps): update pytest requirement from ==8.3.* to ==8.4.*
dependabot[bot] Jun 3, 2025
c865716
chore(deps): update mypy requirement from ==1.15.* to ==1.16.*
dependabot[bot] Jun 4, 2025
4a4d794
fix: sdist failed to install (#199)
epmog Jun 4, 2025
46be6cb
docs: Refine the introduction, move usage into docs dir
mwiebe Jun 5, 2025
f3ccf6f
ci: update to tag based release workflows (#201)
moorec-aws Jun 12, 2025
0068076
docs: Write a worked example of defining a Blender adaptor (#202)
mwiebe Jun 12, 2025
ca7c47a
chore(deps): update python-semantic-release requirement
dependabot[bot] Jun 12, 2025
40606f8
Merge branch 'better_error_subprocess_executable' of https://github.c…
jblagden Aug 21, 2025
9ab0997
Signing unsigned commits
jblagden Aug 21, 2025
44f8c28
Merge branch 'OpenJobDescription:mainline' into better_error_subproce…
jblagden Aug 21, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions src/openjd/adaptor_runtime/process/_logging_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from __future__ import annotations

import logging
import shutil
import signal
import subprocess
import uuid
Expand All @@ -21,7 +22,11 @@


class LoggingSubprocess(object):
"""A process whose stdout/stderr lines are sent to a configurable logger"""
"""A process whose stdout/stderr lines are sent to a configurable logger

Raises:
FileNotFoundError: When the executable the adaptor is set to run cannot be found.
"""

_logger: logging.Logger
_process: subprocess.Popen
Expand Down Expand Up @@ -64,7 +69,27 @@ def __init__(
# In Windows, this is required for signal. SIGBREAK will be sent to the entire process group.
# Without this one, current process will also get the SIGBREAK and may react incorrectly.
popen_params.update(creationflags=subprocess.CREATE_NEW_PROCESS_GROUP) # type: ignore[attr-defined]
self._process = subprocess.Popen(**popen_params)

try:
self._process = subprocess.Popen(**popen_params)

except FileNotFoundError as fnf_error:
# In ManagedProcess we prepend the executable to the list of arguments before creating a LoggingSubprocess
executable = args[0]
exe_path = shutil.which(executable)
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're using shutil.which to resolve the program, it will be better to do this before we call subprocess.Popen, and then modify args[0] to contain the result of that. We found that on Windows, subprocess.Popen and shutil.which can get different answers, so the approach you've implemented could make inaccurate error messages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting - do you recall how we might be able to re-create that? It might be worth a unit test to save future folks from a similar pitfall. I'll get a tweak up as soon as I can - likely tomorrow morning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What a mistake it was to commit to a time I'd get back to this!

I think I've got it all straightened out now though, let me know what you think when you have a chance.


# If we didn't find the executable found by which
if exe_path is not None:
raise FileNotFoundError(
f"Could not find adaptor executable at: {exe_path} using alias {executable}\n"
f"Error:{fnf_error}"
)

raise FileNotFoundError(
f"Could not find the executable associated with the adaptor: {executable}\n"
f"Is the executable on the PATH or in the startup directory?\n"
f"Error:{fnf_error}"
)

if not self._process.stdout: # pragma: no cover
raise RuntimeError("process stdout not set")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ def test_startup_directory(self, startup_dir: str | None, caplog):
def test_startup_directory_empty_posix(self):
"""When calling LoggingSubprocess with an empty cwd, FileNotFoundError will be raised."""
args = ["pwd"]
with pytest.raises(FileNotFoundError) as excinfo:
with pytest.raises(FileNotFoundError) as exc_info:
LoggingSubprocess(args=args, startup_directory="")
assert "[Errno 2] No such file or directory: ''" in str(excinfo.value)
assert "[Errno 2] No such file or directory: ''" in str(exc_info.value)

@pytest.mark.skipif(not OSName.is_windows(), reason="Only run this test in Windows.")
def test_startup_directory_empty_windows(self):
Expand Down Expand Up @@ -151,6 +151,13 @@ def test_log_levels(self, log_level: int, caplog):

assert any(r.message == message and r.levelno == _STDERR_LEVEL for r in records)

def test_executable_not_found(self):
"""When calling LoggingSubprocess with a missing executable, FileNotFoundError will be raised"""
args = ["missing_executable"]
with pytest.raises(FileNotFoundError) as exc_info:
LoggingSubprocess(args=args)
assert "Could not find the executable associated with the adaptor" in str(exc_info.value)


class TestIntegrationRegexHandler(object):
"""Integration tests for LoggingSubprocess"""
Expand Down
Loading