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

PR: Redirecting server stderr and stdout to files #62

Merged
merged 6 commits into from
Jun 23, 2017

Conversation

andfoy
Copy link
Member

@andfoy andfoy commented Jun 22, 2017

Fixes #60

@andfoy andfoy added this to the v0.1.2 milestone Jun 22, 2017
@andfoy andfoy self-assigned this Jun 22, 2017
@andfoy
Copy link
Member Author

andfoy commented Jun 22, 2017

@ccordoba12 Is this correct?

@ccordoba12
Copy link
Member

I'll let you know tomorrow. I'm very tired now ;-)

stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
stdout=self.server_stdout,
stderr=self.server_stderr)
Copy link
Member

Choose a reason for hiding this comment

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

Let's make the server write to a file only if DEV is True. DEV can be imported from config.base and it's True when you start Spyder with bootstrap.py.

@ccordoba12
Copy link
Member

Looks good. Please add a test for this by setting DEV=True in our tests, then verifying that stdout/stderr files are written to disk.

By the way, could we use a single file for both outputs?

@andfoy
Copy link
Member Author

andfoy commented Jun 22, 2017

Yes, it is possible to redirect both outputs to a single file

@ccordoba12
Copy link
Member

Ok, but can we prefix it with something like STDOUT: foo ..., STDERR: baz ... to make it easier to read a single file?

@ccordoba12
Copy link
Member

If it's too much trouble, then we should leave things as they are, though ;-)

@andfoy
Copy link
Member Author

andfoy commented Jun 22, 2017

I think it could be possible if we implement a file wrapper around a single file, but in terms of concurrency, I don't know what would be the expected behaviour

@ccordoba12
Copy link
Member

Then please leave two files. In any case, this feature is just for debugging purposes.

So just add a test for this and we're ready!

@@ -12,5 +12,5 @@
if PYQT5:
from .terminalplugin import TerminalPlugin as PLUGIN_CLASS

VERSION_INFO = (0, 1, 1)
VERSION_INFO = (0, 1, 2)
Copy link
Member

Choose a reason for hiding this comment

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

The dev version should include a dev0, like this

https://github.com/spyder-ide/spyder/blob/master/spyder/__init__.py#L30

@andfoy andfoy merged commit fed774d into spyder-ide:master Jun 23, 2017
@andfoy andfoy deleted the output_redirection branch June 23, 2017 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants