Skip to content
This repository has been archived by the owner on Dec 9, 2018. It is now read-only.

Remove removal of DISPLAY environment variable #74

Merged
merged 2 commits into from
Feb 27, 2017

Conversation

willzhang05
Copy link
Contributor

The issue has to do with the lines 9-10:

if "DISPLAY" in os.environ:
    del os.environ["DISPLAY"]

This seems to remove the DISPLAY environment variable unnecessarily, as on line 50 of xvfbwrapper.py, self.orig_display is set to the value of DISPLAY.

if 'DISPLAY' in os.environ:
    self.orig_display = os.environ['DISPLAY'].split(':')[1]

self.orig_display is checked on line 83, which is where the error occurs. Because of xvfb.py removing the environment variable and self.orig_display being set to the original value, on line 84 when it tries to remove DISPLAY, it has already been removed by xvfb.py, so it throws a KeyError.

if self.orig_display is None:
    del os.environ['DISPLAY']

The solution is simply to remove the removal of the DISPLAY environment variable in xvfb.py, as it is already handled by xvfbwrapper.py

The issue has to do with the two lines:

` if "DISPLAY" in os.environ:
    del os.environ["DISPLAY"]`

This seems to remove the DISPLAY environment variable unnecessarily, as on line 50 of xvfbwrapper.py, self.orig_display is set to the value of DISPLAY. self.orig_display is checked on line 83, which is where the error occurs. Because of xvfb.py removing the environment variable and self.orig_display being set to the original value, on line 84 when it tries to remove DISPLAY, it has already been removed by xvfb.py, so it throws a KeyError.
@philippe2803
Copy link

philippe2803 commented Feb 23, 2017

I was going to do a pull request myself on this one.

@niklasb niklasb merged commit 8a19cd0 into niklasb:master Feb 27, 2017
@niklasb
Copy link
Owner

niklasb commented Feb 27, 2017

Thanks @willzhang05!

@willzhang05 willzhang05 deleted the patch-1 branch February 27, 2017 17:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants