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

Make Plex XML "include" parameters configurable #603

Closed
JonnyWong16 opened this issue Nov 20, 2020 · 7 comments · Fixed by #607
Closed

Make Plex XML "include" parameters configurable #603

JonnyWong16 opened this issue Nov 20, 2020 · 7 comments · Fixed by #607

Comments

@JonnyWong16
Copy link
Collaborator

Describe the issue
The _include parameters when retrieving XML data from Plex should be configurable. It would allow the user to save resources and speed up the response by only retrieving the XML tags/fields that they need.

Example hardcoded _include parameters for ~plexapi.video.Movie:

_include = ('?checkFiles=1&includeExtras=1&includeRelated=1'
'&includeOnDeck=1&includeChapters=1&includePopularLeaves=1'
'&includeConcerts=1&includePreferences=1')

This is especially useful for the checkFiles=1 parameter since this causes the Plex server to hit the storage to check if a file exists (~plexapi.media.MediaPart.exists) and is accessible (~plexapi.media.MediaPart.accessible) which may timeout if cloud storage is being used for the Plex server.

Code snipppets
Example of an unnecessary "heavy" reload.

from plexapi.server import PlexServer
plex = PlexServer('http://localhost:32400', token='xxxxxxxxxxxxxxxxxxxx')

for movie in plex.library.section("Movies").all():
    # This will trigger an automatic reload() of every movie with checkFiles=1
    # even though the exists/accessible values are not needed
    print(movie.subtitleStreams())

Expected behavior
Example of a possible "lighter" reload.

from plexapi.server import PlexServer
plex = PlexServer('http://localhost:32400', token='xxxxxxxxxxxxxxxxxxxx')

for movie in plex.library.section("Movies").all():
    # Manual reload() without checkFiles
    movie.reload(checkFiles=False)
    print(movie.subtitleStreams())

Additional context
Some other XML includes that are available but not shown when viewing the XML through Plex.

  • includeFields=thumbBlurHash adds the thumbBlurHash field to the XML (used by Plexamp)
  • includeFields=artBlurHash adds the artBlurHash field to the XML (used by Plexamp)

Reference: BlurHash.

@Hellowlol
Copy link
Collaborator

This behavior is intentional. Accessing a attribute that isn’t included should trigger a reload with the most possible info.

There are ways to avoid doing heavy reloads.

  1. Limit the scope by using the kwargs for all
  2. Use search as this all filtering happens in pms.
  3. If you know you will trigger a reload you can override by using reload(key) before accessing the attribute

@JonnyWong16
Copy link
Collaborator Author

JonnyWong16 commented Nov 21, 2020

It can still be the default behaviour, but having the includes as customizable parameters is more user friendly.

I am just using subtitleStreams() as an example because that is something that cannot be retrieved from /library/sections/<sectionID>/all.

  1. For my example above, I don't think there is a kwargs that can return subtitle streams using all().
  2. For my example above, search() would only work if you want to filter by subtitleLanguage because that is the only available filter built into Plex (Search Improvements #598). Searching for anything else like subtitleCodec will not work.
  3. This is much cleaner
    movie.reload(checkFiles=False)
    than this
    include = ('?includeExtras=1&includeRelated=1' 
               '&includeOnDeck=1&includeChapters=1&includePopularLeaves=1' 
               '&includeConcerts=1&includePreferences=1')
    movie.reload(movie.key + include)

@blacktwin
Copy link
Collaborator

blacktwin commented Nov 21, 2020

#590 moved the child _include into the parent video class. We could use this single reference to break out params into the known kwargs for reload().

_include is only used to populate _details_keys, so why don't we move these params to reload()

    def reload(self, **kwargs):
        """ Reload the data for this object from self.key. """
        _additionalParams = ('checkFiles', 'includeAllConcerts', 'includeBandwidths', 'includeChapters',
                             'includeChildren', 'includeConcerts', 'includeExtras', 'includeFields',
                             'includeGeolocation', 'includeLoudnessRamps', 'includeMarkers', 'includeOnDeck',
                             'includePopularLeaves', 'includePreferences', 'includeRelated', 'includeRelatedCount',
                             'includeReviews', 'includeStations')
        _allParams = {k: 1 for k in _additionalParams}
        if kwargs:
            for param, value in kwargs.items():
                if param not in _additionalParams:
                    raise NotFound
                elif value not in [0, False]:
                    raise NotFound
                else:
                    _allParams.pop(param)
    key = '?%s' % urlencode(_allParams)
    ....

JonnyWong16 added a commit to JonnyWong16/python-plexapi that referenced this issue Nov 21, 2020
JonnyWong16 added a commit to JonnyWong16/python-plexapi that referenced this issue Nov 21, 2020
JonnyWong16 added a commit to JonnyWong16/python-plexapi that referenced this issue Nov 21, 2020
@JonnyWong16
Copy link
Collaborator Author

I don't think we should have all the includes in reload(). They should remain with the object that it is relevant to. Also, includeFields can have a string value mentioned above instead of the typical integer 1.

Here is my first pass at customizing includes: JonnyWong16@c171aa2.

>>> movie = plex.fetchItem(1)
>>> movie._details_key  # This may be a breaking change since the details key used to be populated by default
''

>>> movie.reload(checkFiles=False)  # Disable an include parameter
<Movie:1:Bridge.of.Spies.2015>
>>> movie._details_key
'/library/metadata/1?includeAllConcerts=1&includeBandwidths=1&includeChapters=1&includeChildren=1&includeConcerts=1&includeExtras=1&includeFields=1&includeFields=thumbBlurHash&includeFields=artBlurHash&includeGeolocation=1&includeLoudnessRamps=1&includeMarkers=1&includeOnDeck=1&includePopularLeaves=1&includePreferences=1&includeRelated=1&includeRelatedCount=1&includeReviews=1&includeStations=1'

>>> movie.reload()  # Default include all parameters
<Movie:1:Bridge.of.Spies.2015>
>>> movie._details_key
'/library/metadata/1?checkFiles=1&includeAllConcerts=1&includeBandwidths=1&includeChapters=1&includeChildren=1&includeConcerts=1&includeExtras=1&includeFields=1&includeFields=thumbBlurHash&includeFields=artBlurHash&includeGeolocation=1&includeLoudnessRamps=1&includeMarkers=1&includeOnDeck=1&includePopularLeaves=1&includePreferences=1&includeRelated=1&includeRelatedCount=1&includeReviews=1&includeStations=1'

>>> movie.reload(includeFields=True)  # Override the include parameter
<Movie:1:Bridge.of.Spies.2015>
>>> movie._details_key               
'/library/metadata/1?checkFiles=1&includeAllConcerts=1&includeBandwidths=1&includeChapters=1&includeChildren=1&includeConcerts=1&includeExtras=1&includeFields=1&includeGeolocation=1&includeLoudnessRamps=1&includeMarkers=1&includeOnDeck=1&includePopularLeaves=1&includePreferences=1&includeRelated=1&includeRelatedCount=1&includeReviews=1&includeStations=1'

>>> movie = plex.fetchItem(1)
>>> movie.isPartialObject()   
True
>>> movie.chapters  # Auto-reload when accessing an attribute that is missing
[]
>>> movie.isPartialObject()
False
>>> movie._details_key
'/library/metadata/1?checkFiles=1&includeAllConcerts=1&includeBandwidths=1&includeChapters=1&includeChildren=1&includeConcerts=1&includeExtras=1&includeFields=1&includeFields=thumbBlurHash&includeFields=artBlurHash&includeGeolocation=1&includeLoudnessRamps=1&includeMarkers=1&includeOnDeck=1&includePopularLeaves=1&includePreferences=1&includeRelated=1&includeRelatedCount=1&includeReviews=1&includeStations=1'

@Hellowlol
Copy link
Collaborator

The same can be done today by using str.replace on the details key. I actually thought about that when I did the changes. I think your approach is better.

Open a PR. We also need to change the way we check if the object is full.

@JonnyWong16
Copy link
Collaborator Author

Do we need to change how we check if it is a full object? The object has been loaded from the API path (details key). Unless you want "full" to mean it has all the include parameters.

@Hellowlol
Copy link
Collaborator

Hellowlol commented Nov 21, 2020

Full should be all params yes. It will only reload on missing attributes anyway.

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 a pull request may close this issue.

3 participants