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

Add **kwargs to LibrarySection.get() for arbitrary query parameters. #554

Closed

Conversation

lidocaineus
Copy link

@lidocaineus lidocaineus commented Aug 14, 2020

Whenever there are title collisions when calling LibrarySection.get(), the first matching element from the query is returned. This patch adds arbitrary key / value pairs to .get() to support optional parameters beyond just "title", enabling more complex searches by adding the additional search parameters to the key; this (hopefully) should enable the correct element to be returned. This is supported already in the Plex XML API but doesn't look to be implemented here. This shouldn't break any existing queries.

Example using "The Killers" originally from 1946 and remade in 1964; the query without additional parameters returns the first element which in this case is the 1946 version:

>>> plex.library.section('Films').get('The Killers')
<Movie:1563:The-Killers>

>>> plex.library.section('Films').get('The Killers', year="1964")
<Movie:1562:The-Killers>

>>> plex.library.section('Films').get('The Killers', year="1946")                                                                     
<Movie:1563:The-Killers>

Other parameters beyond year:

>>> plex.library.section('Films').get('The Killers', guid="com.plexapp.agents.imdb://tt0038669?lang=en")                                                                                                                                                                     
<Movie:1563:The-Killers>

Combinations:

>>> plex.library.section('Films').get('The Killers', year="1946", guid="com.plexapp.agents.imdb://tt0038669?lang=en")                                                                                                                                                                     
<Movie:1563:The-Killers>

@coveralls
Copy link

Coverage Status

Coverage decreased (-5.6%) to 65.375% when pulling 84c84e5 on lidocaineus:LibrarySection.get_kwargs into fe27d76 on pkkid:master.

@Hellowlol
Copy link
Collaborator

Hellowlol commented Aug 15, 2020

Thanks for the PR. If .get can return more then one object you should use search that already supports what this pr implements.

@Hellowlol Hellowlol closed this Aug 15, 2020
@Hellowlol
Copy link
Collaborator

Hellowlol commented Aug 15, 2020

Closing this as i dont think it adds any real benefit. You can use .all(year=1111) or search to do the same thing. Maybe was should add logging if we have more then one item so the user knows it.

@lidocaineus
Copy link
Author

lidocaineus commented Aug 15, 2020

Using search works so that's fine. That said it really should be emphasized that .get() is basically "best effort" and somewhat useless, as any sanity checking on what's returned would better be done using .all() instead.

In other words the function works, but it's usability outside a narrow use case is zero, not to mention it arbitrarily restricts the underlying XML request to "title" (this could've been any of a number of things but someone hardcoded it) instead of exposing its full functionality; my suggestion is it be deprecated and eventually removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants