PR: Fix handling of kernel stderr, and capture stdout and segfaults too (IPython console) #14025

impact27 · 2020-10-20T13:55:05Z

Description of Changes

Sometines, c libraries will print something to the actual stdout and stderr, instead of using the ipykernel ones. This saves these messages to a file and displays them in the console.

Wrote at least one-line docstrings (for any new functions)
Added unit test(s) covering the changes (if testable)

Included a screenshot or animation (if affecting the UI, see Licecap)

Issue(s) Resolved

Fixes #13991
Fixes #9759
Fixes #1922

Depends on #16766
Depends on jupyter/qtconsole#450

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:

pep8speaks · 2020-10-20T13:55:17Z

Hello @impact27! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-11-17 18:55:26 UTC

ccordoba12 · 2020-10-26T20:15:16Z

@impact27, it seems you haven't pushed your changes to spyder-kernels to a corresponding PR in that repo.

impact27 · 2020-10-26T20:22:36Z

@ccordoba12 oups: spyder-ide/spyder-kernels#254

impact27 · 2020-11-09T12:54:49Z

Fixes #1922 as well

impact27 · 2021-04-12T05:32:55Z

Should I leave this on 4.x or move it to 5.x?

ccordoba12 · 2021-10-10T17:09:59Z

This needs to wait until @dalthviz finishes the migration of the IPython console to the new API in PR #16324.

ccordoba12 · 2021-11-04T22:57:19Z

@impact27, could you fix the merge conflicts and try to finish this PR as soon as you can?

Sorry for the rush, but I need the fixes you're doing here to the console shutdown logic to finish PR #16409. That's because our tests are failing with a segfault there, which seems related to the problem @rear1019 was trying to solve on PR #16585.

ccordoba12 · 2021-11-04T22:57:55Z

@impact27, could you fix the merge conflicts and try to finish this PR as soon as you can?

Let me know if you need help with that, so you can keep working on this.

impact27 · 2021-11-06T17:26:45Z

@ccordoba12 Depends on jupyter/qtconsole#505

impact27 · 2021-11-06T17:55:43Z

@ccordoba12 The tests might pass without jupyter/qtconsole#505. When it is merged the last commit can be removed and the tests will be more stable.

spyder/plugins/ipythonconsole/utils/manager.py

impact27 · 2021-11-06T18:09:54Z

spyder/plugins/ipythonconsole/widgets/client.py


    def __del__(self):
-        """Close threads to avoid segfault"""
+        """Close threads to avoid segfault."""


This is likely an issue as well, but not covered in this PR. Don't close spyder while restarting a kernel?

spyder/plugins/ipythonconsole/widgets/main_widget.py

spyder/plugins/outlineexplorer/tests/test_widgets.py

…der-ide/spyder-kernels.git external-deps/spyder-kernels subrepo: subdir: "external-deps/spyder-kernels" merged: "efe0a3f22" upstream: origin: "https://github.com/spyder-ide/spyder-kernels.git" branch: "2.x" commit: "efe0a3f22" git-subrepo: version: "0.4.1" origin: "https://github.com/ingydotnet/git-subrepo" commit: "a04d8c2"

ccordoba12

Thanks @impact27 for this improvement! It looks really good to me, except for the minor comments and suggestions I left below.

Also, I have a question for you: does this new functionality not replace what Wurlitzer does? If that's the case, should we not remove Wurlitzer from the kernel dependencies?

spyder/app/tests/test_mainwindow.py

spyder/plugins/ipythonconsole/widgets/client.py

ccordoba12 · 2021-11-14T20:11:46Z

spyder/plugins/ipythonconsole/widgets/client.py

@@ -478,7 +558,10 @@ def stop_button_click_handler(self):

    def show_kernel_error(self, error):
        """Show kernel initialization errors in infowidget."""
-        InstallerIPythonKernelError(error)
+        self.error_text = error
+        if "issue1666807" not in error:


This looks really ugly and it could lead to problems when building the installer for publishing it.

@mrclary, what do you think about this?

@impact27, what is the significance of "issue1666807"? InstallerIPythonKernelError is perfectly safe to execute under any circumstance, so does not require flow control. InstallerIPythonKernelError will not do anything unless the installer test is running, i.e. the system environment variable INSTALLER_TEST=1; this only occurs on the GitHub server when explicitly testing the macOS application build.

When building the installer, the work in this PR reveals that an error is printer relative to https://bugs.python.org/issue1666807. Basically python complains that some paths are relative and not absolute. I guess this is made to have a portable installer. I don't think the code here creates this error but rather reveals it. I will remove this but I think this will make the installer tests fail.

Error in the Create macOS App Bundle and DMG / macOS App Bundle (Lite) test:

This version of python seems to be incorrectly compiled (internal generated filenames are not absolute). This may make the debugger miss breakpoints. Related bug: http://bugs.python.org/issue1666807

Okay, I've seen this happen recently. It is not related to anything in this PR and this PR did not uncover it. As far as this PR is concerned, that is good news. I was not aware, however, that it was causing the installer test to fail. AFAIK, this may not present an actual issue when using Spyder, but nevertheless needs to be addressed. I'll look into it right away.

Okay, quick clarification. This warning printed in the IPython console began showing up recently, but did not cause the installer to fail. This PR did begin causing the installer to fail; I assume that it was not captured before this PR. I don't know how malicious this warning is, but I'll try to find the root cause.

I could add back the if "issue1666807" not in error: so the tests pass and add an issue to the issue tracker so this can be handled separately?

Unfortunately, I think that is our only option right now. This issue has me stumped. I think we should reopen #16828 rather than creating a new issue.

spyder/plugins/ipythonconsole/widgets/client.py

impact27 · 2021-11-14T22:26:26Z

Also, I have a question for you: does this new functionality not replace what Wurlitzer does? If that's the case, should we not remove Wurlitzer from the kernel dependencies?

This approach wouldn't work for kernels not started by spyder, so Wurlitzer might still be useful.

Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>

ccordoba12

@impact27, I left a tiny suggestion for you, then it should be ready.

Also, please rebase on top of 5.x to get the fix to our tests I made in PR #16845.

spyder/plugins/ipythonconsole/widgets/client.py

Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>

ccordoba12

This is a terrific improvement! Thanks a lot @impact27 for your work on this!

Fixes #13991 Fixes #9759 Fixes #1922

ccordoba12 changed the base branch from master to 4.x October 20, 2020 16:08

ccordoba12 added this to the v4.2.1 milestone Oct 20, 2020

ccordoba12 mentioned this pull request Oct 24, 2020

Uncheking Hide command line output windows generated by the subprocess module is not working #13997

Closed

impact27 mentioned this pull request Oct 25, 2020

Emit kernel_restarted after restarting kernel jupyter/qtconsole#450

Merged

ccordoba12 modified the milestones: v4.2.1, v4.2.2 Nov 12, 2020

ccordoba12 modified the milestones: v4.2.2, 4.x (missing) Feb 1, 2021

kbauer mentioned this pull request Feb 18, 2021

Output of os.system, subprocess.call goes to wrong console #14783

Closed

10 tasks

ccordoba12 changed the base branch from 4.x to 5.x October 10, 2021 17:03

impact27 force-pushed the std_files branch from ebabfca to 765b5e7 Compare October 13, 2021 15:42

impact27 mentioned this pull request Nov 3, 2021

PR: Fix QProcess error messages in Completions plugin #16585

Merged

impact27 force-pushed the std_files branch 2 times, most recently from 737d9b5 to a0e9b6a Compare November 5, 2021 15:57

ccordoba12 modified the milestones: 4.x, v5.2.0 Nov 5, 2021

impact27 commented Nov 6, 2021

View reviewed changes

spyder/plugins/ipythonconsole/utils/manager.py Outdated Show resolved Hide resolved

impact27 commented Nov 6, 2021

View reviewed changes

spyder/plugins/ipythonconsole/widgets/main_widget.py Outdated Show resolved Hide resolved

impact27 commented Nov 6, 2021

View reviewed changes

spyder/plugins/outlineexplorer/tests/test_widgets.py Outdated Show resolved Hide resolved

impact27 mentioned this pull request Nov 8, 2021

stderr buffer doesn't clear between runs. #16743

Closed

ccordoba12 mentioned this pull request Nov 8, 2021

multiprocessing failing when function contains a class on the same file #16696

Closed

Quentin Peter and others added 3 commits November 10, 2021 13:36

tests

2895ea9

Fix std show

11d4357

impact27 force-pushed the std_files branch from 39d7c78 to 11d4357 Compare November 10, 2021 12:54

impact27 added 2 commits November 13, 2021 09:50

Merge branch '5.x' into std_files

0928866

Update .gitrepo

f1734c7

ccordoba12 reviewed Nov 14, 2021

View reviewed changes

ccordoba12 changed the title ~~PR: Fix stdout and stderr handling~~ PR: Fix handling of kernel stderr, and capture stdout and segfaults too (IPython console) Nov 14, 2021

mrclary reviewed Nov 14, 2021

View reviewed changes

spyder/plugins/ipythonconsole/widgets/client.py Outdated Show resolved Hide resolved

impact27 and others added 4 commits November 14, 2021 22:28

Apply suggestions from code review

6427fe5

Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>

Move StdFile

ccc66e4

revert fix for issue1666807

db8f7f6

Fix path

5bfe10a

mrclary mentioned this pull request Nov 15, 2021

PR: Constrain IPython to be less than 7.28.0 for macOS installer #16829

Merged

Merge remote-tracking branch 'upstream/5.x' into std_files

6b0647a

mrclary mentioned this pull request Nov 16, 2021

Python seems to be incorrectly compiled for macOS installer #16828

Closed

skip error for macos installer

d364920

ccordoba12 reviewed Nov 17, 2021

View reviewed changes

spyder/plugins/ipythonconsole/widgets/client.py Show resolved Hide resolved

ccordoba12 assigned impact27 Nov 17, 2021

impact27 and others added 2 commits November 17, 2021 19:49

Update spyder/plugins/ipythonconsole/widgets/client.py

5fc1599

Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>

Merge remote-tracking branch 'upstream/5.x' into std_files

b4dfd7d

ccordoba12 approved these changes Nov 17, 2021

View reviewed changes

ccordoba12 merged commit f20053f into spyder-ide:5.x Nov 17, 2021

ccordoba12 added a commit that referenced this pull request Nov 17, 2021

Merge from 5.x: PR #14025

f263265

Fixes #13991 Fixes #9759 Fixes #1922

mrclary mentioned this pull request Nov 18, 2021

PR: Filter 'This version of python seems to be incorrectly compiled...' pydev_log.critical message #16849

Merged

impact27 mentioned this pull request Dec 1, 2021

Debugger message bug gets printed in the IPython Console #16927

Closed

mrclary mentioned this pull request Dec 2, 2021

Qt5 backend hangs after clicking the edit axis toolbar icon #16946

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR: Fix handling of kernel stderr, and capture stdout and segfaults too (IPython console) #14025

PR: Fix handling of kernel stderr, and capture stdout and segfaults too (IPython console) #14025

impact27 commented Oct 20, 2020 •

edited by ccordoba12

Loading

pep8speaks commented Oct 20, 2020 •

edited

Loading

ccordoba12 commented Oct 26, 2020

impact27 commented Oct 26, 2020

impact27 commented Nov 9, 2020

impact27 commented Apr 12, 2021

ccordoba12 commented Oct 10, 2021

ccordoba12 commented Nov 4, 2021

ccordoba12 commented Nov 4, 2021 •

edited

Loading

impact27 commented Nov 6, 2021

impact27 commented Nov 6, 2021

impact27 Nov 6, 2021

ccordoba12 left a comment

ccordoba12 Nov 14, 2021

mrclary Nov 14, 2021

impact27 Nov 14, 2021

impact27 Nov 14, 2021

mrclary Nov 15, 2021

mrclary Nov 15, 2021

impact27 Nov 16, 2021

mrclary Nov 16, 2021

impact27 commented Nov 14, 2021

ccordoba12 left a comment

ccordoba12 left a comment

PR: Fix handling of kernel stderr, and capture stdout and segfaults too (IPython console) #14025

PR: Fix handling of kernel stderr, and capture stdout and segfaults too (IPython console) #14025

Conversation

impact27 commented Oct 20, 2020 • edited by ccordoba12 Loading

Description of Changes

Issue(s) Resolved

Affirmation

pep8speaks commented Oct 20, 2020 • edited Loading

Comment last updated at 2021-11-17 18:55:26 UTC

ccordoba12 commented Oct 26, 2020

impact27 commented Oct 26, 2020

impact27 commented Nov 9, 2020

impact27 commented Apr 12, 2021

ccordoba12 commented Oct 10, 2021

ccordoba12 commented Nov 4, 2021

ccordoba12 commented Nov 4, 2021 • edited Loading

impact27 commented Nov 6, 2021

impact27 commented Nov 6, 2021

Choose a reason for hiding this comment

ccordoba12 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

impact27 commented Nov 14, 2021

ccordoba12 left a comment

Choose a reason for hiding this comment

ccordoba12 left a comment

Choose a reason for hiding this comment

impact27 commented Oct 20, 2020 •

edited by ccordoba12

Loading

pep8speaks commented Oct 20, 2020 •

edited

Loading

ccordoba12 commented Nov 4, 2021 •

edited

Loading