PR: Pass TMPDIR env var to kernels (IPython console)
#22382
Conversation
…s.git --branch=remove-tmpdir --update --force external-deps/spyder-kernels subrepo: subdir: "external-deps/spyder-kernels" merged: "3e5998a1b4" upstream: origin: "https://github.com/ccordoba12/spyder-kernels.git" branch: "remove-tmpdir" commit: "3e5998a1b4" git-subrepo: version: "0.4.9" origin: "https://github.com/ingydotnet/git-subrepo" commit: "ea10886"
…r-kernels.git --branch=master --update --force external-deps/spyder-kernels subrepo: subdir: "external-deps/spyder-kernels" merged: "66d58eb0be" upstream: origin: "https://github.com/spyder-ide/spyder-kernels.git" branch: "master" commit: "66d58eb0be" git-subrepo: version: "0.4.9" origin: "https://github.com/ingydotnet/git-subrepo" commit: "ea10886" [ci skip]
ca93b42 to
06e9ccb
Compare
| # This env var avoids polluting the OS default temp directory with | ||
| # files generated by `conda run`. It's removed in the kernel after | ||
| # initialization to not affect users code. | ||
| "TMPDIR": get_temp_dir(), |
There was a problem hiding this comment.
This overwrites the existing TMPDIR on macOS, which then is removed in the kernel environment. This may cause issues for users since this is a standard environment variable that is expected to point to the system temporary directory, /var/folders/5v/28jqvwxs2cd5fj93gvwnykdrqc926z/T/ in my case. I believe this variable is also standard on Linux.
We should consider using SPY_TMPDIR instead.
There was a problem hiding this comment.
Hmm... seems TMPDIR is still in the IPython console environment, but points to the spyder-<user> directory. I agree that this is where we should put spyder-kernels connection files, but this may still be an issue if users are expecting the standard temporary directory location, particularly if they produce a lot of temp files that would pollute the spyder-<user> directory.
There was a problem hiding this comment.
Sorry, I didn't know TMPDIR is a default env var on macOS. I'll fix that by using another env var (TEMP or TMP can also be set to change the root directory used by the tempfile module).
There was a problem hiding this comment.
Unfortunately, I don't think using another environment variable that tempfile recognizes will work. The problem will be when a user uses tempfile in the IPython console (their own code), it will return <system temp directory>/spyder-<user> instead of just <system temp directory>. For me this value is tempfile.tempdir = '/var/folders/5v/28jqvwxs2cd5fj93gvwnykdrqc926z/T/spyder-rclary'. This means that instead of kernel connection files polluting <system temp directory>, then a user's temp files will pollute <system temp directory>/spyder-<user>.
I think we should append spyder-<user> to tempfile.tempdir when we need it, like we do for the update manager, rather than change the value of tempfile.tempdir.
There was a problem hiding this comment.
You're right. That's why I decided to follow your initial advice and use SPY_TMPDIR to save the system TMPDIR on the Spyder side, send it to the kernel and restore it there after it's started.
That's all done in PR #22394. I added a test to check that it preserves TMPDIR on Mac and also verified manually that that's the case. So, I'm going to merge it.
There was a problem hiding this comment.
I think we should append spyder- to tempfile.tempdir when we need it, like we do for the update manager, rather than change the value of tempfile.tempdir.
That's not possible because it's basically out of our control. The temp files I'm trying to place in another directory are generated by conda run. And even if we submitted a PR and changed things there, it won't apply for older conda versions.
There was a problem hiding this comment.
That's not possible because it's basically out of our control. The temp files I'm trying to place in another directory are generated by
conda run. And even if we submitted a PR and changed things there, it won't apply for older conda versions.
Aahh, okay. I thought the temp files (and location) would be controlled by spyder-kernels; that's interesting that conda run would determine this.
Description of Changes
conda run.TMPDIRenv var after initialization spyder-kernels#498Issue(s) Resolved
Fixes #
Affirmation
By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.
I certify the above statement is true and correct: