Skip to content

Switch to using Client from twilio.rest rather than the deleted TwillioRestClient#17885

Merged
balloob merged 2 commits intohome-assistant:rcfrom
rohankapoorcom:twilio-client-change-master
Oct 27, 2018
Merged

Switch to using Client from twilio.rest rather than the deleted TwillioRestClient#17885
balloob merged 2 commits intohome-assistant:rcfrom
rohankapoorcom:twilio-client-change-master

Conversation

@rohankapoorcom
Copy link
Copy Markdown
Member

Description:

Twilio changed the class that needs to be used from TwilioRestClient to Client. Source.

This broke after the twilio client was upgraded in #17424 and an API change was missed.

Related issue (if applicable): fixes #17871

This one is affecting 0.81.x and should be shipped in the next bugfix release. However, I'm not sure how to do that since the component itself was rewritten in #17715 (so the files are not in the same place in dev and master). This pull request is targeted at master in case that helps for the hotfix and #17883 is targeted at dev which should fix it for 0.82.0

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

@homeassistant homeassistant added integration: twilio merging-to-master This PR is merging into the master branch and should probably change the branch to `dev`. small-pr PRs with less than 30 lines. cla-signed labels Oct 27, 2018
@ghost ghost added the in progress label Oct 27, 2018
Copy link
Copy Markdown

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

@bachya
Copy link
Copy Markdown
Contributor

bachya commented Oct 27, 2018

Make sure to base your PR against dev, not master.

@rohankapoorcom
Copy link
Copy Markdown
Member Author

@bachya please read the last paragraph before the checklist. There's one pull request against master and another against dev because the code got moved around in dev and we need to hotfix the release as well as fix dev.

Please let me know if there's a better approach for this.

@balloob balloob changed the base branch from master to rc October 27, 2018 19:35
@balloob
Copy link
Copy Markdown
Member

balloob commented Oct 27, 2018

Changing to RC. Will probably be nasty when we merge master back in to dev, but we'll survive.

@balloob balloob merged commit a0d4595 into home-assistant:rc Oct 27, 2018
@ghost ghost removed the in progress label Oct 27, 2018
@rohankapoorcom rohankapoorcom deleted the twilio-client-change-master branch October 27, 2018 20:30
@rohankapoorcom
Copy link
Copy Markdown
Member Author

Thanks! I'm expecting it to be annoying, but couldn't think of a way to avoid it. Let me know if I can help with the merge.

@balloob balloob mentioned this pull request Oct 27, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Feb 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed integration: twilio merging-to-master This PR is merging into the master branch and should probably change the branch to `dev`. small-pr PRs with less than 30 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants