set sticky bit on connection files #201
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Alternative: we could set the sticky bit on the runtime_dir itself. |
|
The spec only mentions setting the sticky bit on files, and whatever does the cleanup is probably running as root, so it wouldn't be bound by the sticky bit. So let's stick with doing it on the file rather than the directory. |
jupyter_client/connect.py
Outdated
| @@ -135,6 +136,21 @@ def write_connection_file(fname=None, shell_port=0, iopub_port=0, stdin_port=0, | |||
| with open(fname, 'w') as f: | |||
| f.write(json.dumps(cfg, indent=2)) | |||
|
|
|||
| if hasattr(stat, 'S_ISVTX'): | |||
| # set the sticky bit to avoid periodic cleanup | |||
| permissions = os.stat(fname).st_mod | |||
|
The error in #199 suggests that the runtime_dir was cleaned up because writing a new file should only fail if the directory doesn't exist. |
avoids periodic cleanup of runtime directory
minrk
force-pushed
the
sticky-connection-file
branch
from
3e5d2f9
to
97a0f83
Compare
|
Now that I think about it, since #199 suggests that the directory is what was cleaned up, we may need something directory-related. Either ensure the directory exists when we try to write a connection file, or set the sticky bit on our runtime_dir. |
|
Or do something non-sticky like call |
|
Ah, right, we make a subdirectory inside the runtime dir. I'd forgotten that. Yes, in that case we probably need this sticky bit on the jupyter directory. |
in addition to connection file to avoid cleanup of empty dir during long-idle servers
minrk
force-pushed
the
sticky-connection-file
branch
from
10b7ac3
to
aca211c
Compare
connection file is in CWD, no need for sticky bits
|
Finally got back to this one. It now sets the sticky bit on the directory containing the connection file, as well. |
blink1073
approved these changes
Feb 7, 2017
|
@minrk it seems like setting the sticky bit consistently fails on macOS because of a permission issue with the |
|
I am seeing the same error on Mac. |
|
We're already catching the error and issuing a warning, which is what you see. Maybe we should skip the warning on Mac since it probably doesn't matter. |
|
I frankly do not know if it matters but everything appears to be ok after the warning. The warning is pretty low level and scary so it is perhaps better to remove it (at least on mac). |
|
It shouldn't matter - setting the sticky bit is part of an XDG specification (XDG_RUNTIME_DIR), which as far as I know is only used on free desktop systems (Linux, BSD, etc.). |
|
I run a command to rexecute a jupyter notebook in my Makefile and the output warning looks a bit scary for those unfamiliar. I'd give a +1 for either not setting the sticky bit on mac or suppressing the warning on macos. |
|
@AlJohri Can you show the I'm okay skipping this on mac, but it's a bit surprising because I don't get an permission errors doing this, so I'd like to understand it, as well. |
|
sure, I think |
|
Strange. That looks like you do have permission to set permissions on /var/folders/c5/sxpknfp571v3ydglf4305g9m0000gn/T, which is what's failing above. Can you run: import os
import stat
path = '/var/folders/c5/sxpknfp571v3ydglf4305g9m0000gn/T'
permissions = os.stat(path).st_mode
new_permissions = permissions | stat.S_ISVTX
os.chmod(path, new_permissions) |
|
|
And what about: ? |
|
|
Weird! All evidence points to you having permission to set the bit, I don't know why it wouldn't work. In any case, #286 should suppress the warning in this particular case. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
avoids periodic cleanup of runtime directory
closes #199