Skip to content

Temporary Plex play_media workaround#33542

Merged
balloob merged 3 commits into
home-assistant:devfrom
jjlawren:plex_playmedia_workaround
Apr 2, 2020
Merged

Temporary Plex play_media workaround#33542
balloob merged 3 commits into
home-assistant:devfrom
jjlawren:plex_playmedia_workaround

Conversation

@jjlawren
Copy link
Copy Markdown
Contributor

@jjlawren jjlawren commented Apr 2, 2020

Proposed change

Playing media to certain Plex clients fails silently. A fix is available but a plexapi release with those fixes has not yet been published. Until then we can pass that same parameter via the **kwargs on the library's method.

This patch should be removed when upgrading to a new plexapi release.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

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

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

@jjlawren jjlawren mentioned this pull request Apr 2, 2020
Closed
@jjlawren jjlawren added this to the 0.108.0 milestone Apr 2, 2020
@springstan
Copy link
Copy Markdown
Member

springstan commented Apr 2, 2020

I do not think that this temporary workaround is a good idea, we should wait for the new release of plexapi.

@jjlawren
Copy link
Copy Markdown
Contributor Author

jjlawren commented Apr 2, 2020

I requested a new release weeks ago, unfortunately still not available. I also found some issues in the bleeding edge release of the dependency which relies on additional unmerged fixes.

This is a non-invasive change for a problem report that we receive quite often. I see your point but I'd really like to get things working for more people and would rather not wait another full release cycle.

springstan
springstan reviewed Apr 2, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@springstan springstan left a comment

Choose a reason for hiding this comment

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

I see your point as well and understand that it can be frustrating to wait for a new release.

However, let's make sure this workaround is implemented properly and then I think we can include this in 0.108.

Comment thread homeassistant/components/plex/media_player.py Outdated
springstan
springstan reviewed Apr 2, 2020
View reviewed changes
Comment thread homeassistant/components/plex/const.py Outdated
Comment thread homeassistant/components/plex/media_player.py Outdated
springstan
springstan reviewed Apr 2, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@springstan springstan left a comment

Choose a reason for hiding this comment

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

Just some minor things to make this compatible with the style guidelines.

@Insension Insension mentioned this pull request Apr 2, 2020
Closed
3 tasks
@balloob balloob merged commit 55870ae into home-assistant:dev Apr 2, 2020
@balloob
Copy link
Copy Markdown
Member

balloob commented Apr 2, 2020

Oops I somehow missed Springstan's review 👎 Sorry about that. Please address his review in a subsequent PR.

balloob pushed a commit that referenced this pull request Apr 3, 2020
@jjlawren @balloob
Temporary Plex play_media workaround (#33542)
a8da039 
* Temporary playMedia() workaround on plexapi 3.3.0

* Use constants for strings

* Style cleanup
@jjlawren jjlawren mentioned this pull request Apr 3, 2020
Merged
20 tasks
@jjlawren jjlawren deleted the plex_playmedia_workaround branch April 3, 2020 01:53
balloob pushed a commit that referenced this pull request Apr 3, 2020
@jjlawren
Plex followup to #33542 (#33558)
d98171e
balloob pushed a commit that referenced this pull request Apr 3, 2020
@jjlawren @balloob
Plex followup to #33542 (#33558)
ff3bfad
@lock lock Bot locked and limited conversation to collaborators Apr 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@springstan springstan springstan left review comments

Assignees

No one assigned

Labels

by-code-owner cherry-picked cla-signed integration: plex small-pr PRs with less than 30 lines.

Projects

None yet

Milestone

0.108.0

Development

Successfully merging this pull request may close these issues.

Plex play_media service does not work

4 participants

@jjlawren @springstan @balloob @homeassistant