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

Plex: Add exception handler when connection fails #8179

Merged
merged 3 commits into from
Jul 13, 2017

Conversation

abmantis
Copy link
Contributor

Description:

Add exception handler for when the connection fails (server is down).

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass

@mention-bot
Copy link

@abmantis, thanks for your PR! By analyzing the history of the files in this pull request, we identified @JesseWebDotCom, @fabaff and @tomduijf to be potential reviewers.

@@ -145,6 +145,10 @@ def update_devices():
_LOGGER.error("Could not connect to plex server at http://%s",
host)
return
except requests.exceptions.ConnectionError:
_LOGGER.error("Could not connect to plex server at http://%s",
host)
Copy link
Member

Choose a reason for hiding this comment

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

You might want to consider catching RequestException, see http://docs.python-requests.org/en/latest/user/quickstart/#errors-and-exceptions .

Other than that, I'd recommend using except .. as:

except X as ex:
    _LOGGER.error("asdf: %s" % ex)

to give a hint to the user about what type of error happened.

@@ -145,6 +145,10 @@ def update_devices():
_LOGGER.error("Could not connect to plex server at http://%s",
host)
return
except requests.exceptions.RequestException as ex:
Copy link
Member

Choose a reason for hiding this comment

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

I think that you should remove the catching of OSError, because that never happens.

@balloob
Copy link
Member

balloob commented Jul 7, 2017

Any plans to finish this PR?

@abmantis
Copy link
Contributor Author

abmantis commented Jul 7, 2017

Yes. I'll probably be able to get back to this later today or tomorrow.

@balloob
Copy link
Member

balloob commented Jul 13, 2017

Bueno 🐬

@balloob balloob merged commit 2eeeb90 into home-assistant:dev Jul 13, 2017
@balloob balloob mentioned this pull request Jul 13, 2017
@abmantis abmantis deleted the plex_no_connection_handle branch August 11, 2017 23:01
dethpickle pushed a commit to dethpickle/home-assistant that referenced this pull request Aug 18, 2017
* add exception handler when connection fails

* plex: improve exception handling

* remove uneeded exception handler
@home-assistant home-assistant locked and limited conversation to collaborators Dec 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants