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

Fix/oauth fail due to blocked user agent #143

Conversation

xernaj
Copy link
Contributor

@xernaj xernaj commented Dec 19, 2019

Fixes #142 #121

  • Update an old user agent in header that was blocked so oauth token could not be retrieved
  • OAuth bearer token must be in get calls (haven't tested posts/puts)
  • HTTP 429 (rate limited) could possibly still occur due to the amount of retries and loops in the component. I think ring have enabled extra restrictions.
  • linter fixes

Not done:

  • oauth token should be cached rather than retrieved per call. This could potentially appear as an error in future.
  • HTTP 429 check rather than adding a sleep() between retries

tests/test_ring.py Outdated Show resolved Hide resolved
ring_doorbell/__init__.py Outdated Show resolved Hide resolved
ring_doorbell/__init__.py Outdated Show resolved Hide resolved
ring_doorbell/__init__.py Outdated Show resolved Hide resolved
ring_doorbell/__init__.py Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Dec 19, 2019

Coverage Status

Coverage decreased (-0.3%) to 63.241% when pulling 627a596 on xernaj:fix/oauth-fail-due-to-blocked-user-agent into 6d2368b on tchellomello:master.

ring_doorbell/__init__.py Show resolved Hide resolved
ring_doorbell/__init__.py Show resolved Hide resolved
tests/test_ring.py Outdated Show resolved Hide resolved
@xernaj xernaj mentioned this pull request Dec 19, 2019
Copy link

@brama1966 brama1966 left a comment

Choose a reason for hiding this comment

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

works perfectly

@tchellomello tchellomello self-assigned this Dec 20, 2019
Copy link
Member

@tchellomello tchellomello left a comment

Choose a reason for hiding this comment

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

Hello @xernaj, thanks for jumping on this so quickly and providing a fix. Thanks also for all the community efforts as well.

@xernaj I've added some comments on your PR and please let me know what you think about it.

Thanks again for your help!!

ring_doorbell/doorbot.py Outdated Show resolved Hide resolved
ring_doorbell/stickup_cam.py Outdated Show resolved Hide resolved
ring_doorbell/__init__.py Outdated Show resolved Hide resolved
Copy link
Member

@tchellomello tchellomello left a comment

Choose a reason for hiding this comment

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

Thanks @xernaj I've added some comments for the time.sleep(5) line, then it looks to be merged!

move sleep to variable wait when calling authenticate

adjust some indenting
Copy link
Member

@tchellomello tchellomello left a comment

Choose a reason for hiding this comment

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

@xernaj after the pro tip review from the master @balloob, I want to apologize and ask if you want to create a new PR with the if statements you had before. I'm going ahead and merging this so we can have a hotfix in HA. @xernaj are you also planning to submit a PR to home assistant project?

@tchellomello tchellomello merged commit 8bd82cb into python-ring-doorbell:master Dec 20, 2019
@tchellomello tchellomello mentioned this pull request Dec 20, 2019
@xernaj
Copy link
Contributor Author

xernaj commented Dec 20, 2019

Thanks @tchellomello and @balloob, will look at a PR later for if changes though I don't see much value yet as it's a non functional change. As for PR for home assistant project, if someone else has the time, please go ahead, otherwise I can raise one in the next 24-48hrs.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 18, 2024
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.

Ring API stopped working
6 participants