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

Don't wait forever for notebook server to launch/die for tests #4773

Merged
merged 3 commits into from Jan 9, 2014

Conversation

takluyver
Copy link
Member

Should turn occasional test hangs (e.g. https://s3.amazonaws.com/archive.travis-ci.org/jobs/16615179/log.txt ) into straightforward errors.

Should turn occasional hangs into straightforward errors.

@classmethod
def wait_until_dead(cls):
"""Wait for the server to stop getting requests after shutdown"""
url = 'http://localhost:%i/api/notebooks' % cls.port
while True:
for _ in range(300):
Copy link
Member

Choose a reason for hiding this comment

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

this one can just wait until notebook.poll() is not None, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, I've simplified it now.

@ivanov
Copy link
Member

ivanov commented Jan 8, 2014

in general, I'd say it's good practice to avoid having bare naked constants in code like the 300 used twice here without a name or an explanation. If you just add a

MAX_WAITTIME = 30   # seconds to wait for notebook server to start
POLL_INTERVAL = 0.1 # time between attempts

Then you can changes your for loops to be for _ in MAX_WAITTIME/POLL_INTERVAL:

But since this is just test code, I'm not too bothered by it.

@ivanov
Copy link
Member

ivanov commented Jan 8, 2014

otherwise 👍

@@ -52,6 +55,7 @@ def setup_class(cls):
sys.executable, '-c',
'from IPython.html.notebookapp import launch_new_instance; launch_new_instance()',
'--port=%d' % cls.port,
'--port-retries=0',
Copy link
Member

Choose a reason for hiding this comment

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

is this "don't retry" or "retry indefinitely"

Copy link
Member

Choose a reason for hiding this comment

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

the first - it is "retry zero times"

@takluyver
Copy link
Member Author

Thanks for clarifying the code, Paul ;-)

ivanov added a commit that referenced this pull request Jan 9, 2014
Don't wait forever for notebook server to launch/die for tests
@ivanov ivanov merged commit 4aa4a21 into ipython:master Jan 9, 2014
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Don't wait forever for notebook server to launch/die for tests
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

3 participants