notebook: don’t filter polled instances by PID #4407
Merged
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.
Summary:
When the
%tensorboard
cell magic is invoked, we compute a cache keyfor the “hermetic environment”, primarily args to
%tensorboard
and theworking directory. We first check whether any running TensorBoard
instances match that cache key, and launch a new instance if none do.
But then, while polling for the new instance to have launched, we had a
different matching criterion, checking for a process ID match instead of
a cache key match.
The idea was that “is this TensorBoard instance’s PID equal to the PID
of the subprocess that we just spawned?” would be a more reliable check.
But on Windows ((╯°□°)╯︵ ┻━┻) this is not the case, presumably because
the
tensorboard
console script has some kind of wrapper process incertain versions of Python. This manifested as “
%tensorboard
alwaystimes out on the first invocation, but works immediately when I invoke
it again”, since invoking it again triggers the cache key check rather
than the PID check. So we now just check by cache key in all cases, and
the logic is consistent, if a bit less precise overall.
Fixes #4300.
Test Plan:
Still works for me on Linux, with both new and existing TensorBoard
processes across multiple (concurrent) cache keys. @stephanwlee can
repro the bug and fix on Windows with Python 3.8.
wchargin-branch: notebook-poll-no-pid-filter