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

Fail eagerly to wait for the idle status if we have reached a "final" status #81

Merged
merged 3 commits into from
Dec 19, 2015

Conversation

msftristew
Copy link
Contributor

  • Rename wait_for_status() to wait_for_idle() and adjust parameter list appropriately. If we ever need to wait for a status besides idle (seems unlikely?), then this can be refactored back out again.
  • Throw a LivyUnexpectedError if the Livy session goes into error or dead.

@msftristew
Copy link
Contributor Author

  1. Output will look similar to when a LivyClientTimeoutError is thrown.
  2. I am unsure a) if we can get useful error info from Livy when we get this kind of error and b) how we might capture that useful error info and display it to the user. This merits further discussion. However, the current error message is "good enough" IMO.

@@ -0,0 +1,3 @@
class LivyUnexpectedError(Exception):
"""An exception that will be shown if some unexpected error happens on the Livy side."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we rename to LivyUnexpectedFinalStatusErrors? This seems too broad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My idea was that this exception could be used for other uses besides just an unexpected final status but this is OK too.

@aggFTW
Copy link
Contributor

aggFTW commented Dec 19, 2015

All sessions include a logs field. Include logs in message If exception is in error.

@aggFTW
Copy link
Contributor

aggFTW commented Dec 19, 2015

Please include Closes #66 when addressing a bug. That will close it automatically on merge.

@msftristew
Copy link
Contributor Author

What is the name of the extra field?

@aggFTW
Copy link
Contributor

aggFTW commented Dec 19, 2015

Lgtm! Merging

aggFTW added a commit that referenced this pull request Dec 19, 2015
Fail eagerly to wait for the idle status if we have reached a "final" status
@aggFTW aggFTW merged commit 3c0230b into jupyter-incubator:master Dec 19, 2015
@msftristew msftristew deleted the finalstatus branch December 21, 2015 22:01
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.

3 participants