Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ensure executable paths are absolute #129

Merged
merged 4 commits into from
Jun 7, 2023
Merged

Conversation

minrk
Copy link
Member

@minrk minrk commented Jun 7, 2023

systemd requires absolute paths for executables in order to resolve as expected.

Relative path executables do not take $PATH env into account when resolving the executable, only after launching the process, so systemd-run bash -c exe works, while just systemd-run exe will not.

The default executable for a Spawner is the relative jupyterhub-singleuser, assuming $PATH will be resolved.

Fixes the failure in jupyterhub/the-littlest-jupyterhub#915

systemd requires absolute paths for executables.

Relative path executables do not take $PATH env into account when resolving the executable.
systemdspawner/systemd.py Outdated Show resolved Hide resolved
systemdspawner/systemd.py Outdated Show resolved Hide resolved
systemdspawner/systemd.py Outdated Show resolved Hide resolved
Copy link
Member

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

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

LGTM I think, if this seems ready then I figure we go for a 1.0.1 release with this as well.

@minrk
Copy link
Member Author

minrk commented Jun 7, 2023

@consideRatio added one more push with clearer comments, so should be all set from me

thanks everyone for the quick review!

@consideRatio consideRatio merged commit 5b91309 into jupyterhub:main Jun 7, 2023
@consideRatio
Copy link
Member

@minrk @manics @behrmann thank you for solving this!!

@consideRatio
Copy link
Member

I'll create a changelog PR soon for 1.0.1, but I'd like to squeeze in some debugging log messages as well first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants