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

Bump amcrest to 1.2.6 & use new exceptions #22040

Merged
merged 3 commits into from Mar 14, 2019

Conversation

Projects
None yet
5 participants
@pnbruckner
Copy link
Contributor

pnbruckner commented Mar 14, 2019

Description:

Per @amelchio's comment/suggestion in PR #21664:

"It's a shame the library does not handle these exceptions and raises its own instead."

the amcrest package was changed to handle exceptions coming from urllib3 & requests and raise its own unique exceptions instead to make it easier on the "user" (in this case, the amcrest homeassistant component.) Therefore this PR bumps the amcrest package version and changes the amcrest homeassistant component accordingly.

Related issue (if applicable): None

Pull request in home-assistant.io with documentation (if applicable): None

Example entry for configuration.yaml (if applicable):

Does not affect configuration.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

If the code communicates with devices, web services, or third-party tools:

  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.

pnbruckner added some commits Mar 13, 2019

@cgtobi

cgtobi approved these changes Mar 14, 2019

Copy link
Collaborator

cgtobi left a comment

Assuming Travis passes, LGTM.

@pnbruckner pnbruckner changed the title Amcrest exceptions Bump amcrest to 1.2.6 & use new exceptions Mar 14, 2019

@amelchio
Copy link
Member

amelchio left a comment

This is already a nice cleanup but let me suggest going even further :)

Show resolved Hide resolved homeassistant/components/amcrest/__init__.py Outdated
Show resolved Hide resolved homeassistant/components/amcrest/camera.py Outdated
@pnbruckner

This comment has been minimized.

Copy link
Contributor Author

pnbruckner commented Mar 14, 2019

Any idea what the Travis errors are about?

@balloob balloob merged commit 8d2d71c into home-assistant:dev Mar 14, 2019

3 checks passed

Hound No violations found. Woof!
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@wafflebot wafflebot bot removed the in progress label Mar 14, 2019

@pnbruckner pnbruckner deleted the pnbruckner:amcrest-exceptions branch Mar 14, 2019

@hunterjm hunterjm referenced this pull request Mar 16, 2019

Merged

Bump amcrest to 1.2.7 for correct RTSP port #22099

4 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.