Accessing optional attributes results in an unnecessary reload

[Montellese]

Describe the issue

This is not a bug but a feature / improvement request.

I'm trying to retrieve all items from a library section e.g. all movies. Then I'm collecting all the information available for the retrieved movies which requires a reload for some of the attributes like Movie.roles because not all roles are present in the XML data received from the /library/sections/{id}/all endpoint. Since I'm not interested in all the extra stuff like On Deck, Related etc. I wanted to make use of the possibility to customize the used includeXYZ in PlexObject.reload() as introduced in #607. While the customized call to PlexObject.reload() works fine, in the end plexapi often ends up performing a full reload() anyway because I try to access an attribute of Movie or Video which is optional in the XML response like lastViewedAt (which is only provided if the movie has been viewed at least once). The problem is that for all unviewed movies Movie.lastViewedAt will be None and the Movie object will only be a partial object because I did a partial reload() so the logic in PlexObject.reload() will perform a full reload in addition to the customized partial reload() I executed before.

https://github.com/pkkid/python-plexapi/blob/2bde2344084721d7015167af4762d3bc397fe5d2/plexapi/base.py#L426-L442

Is there any way we can add a list of attributes to the specific media classes like Video, Movie etc. which contains all attributes which don't require a full reload() even if they are None or []?

PS: This does not only apply to lastViewedAt but also to collections, originalTitle and others.

Code snippets

The following code snippet first performs a customized reload() and then internally a full reload() if lastViewedAt is not set because the video has never been played / watched:

item.reload(includeOnDeck=False, includeRelated=False, includeReviews=False)
print(f"{item.title} last viewed at {item.lastViewedAt}")

Expected behavior

Only the customized reload() actually reloads the item while accessing lastViewedAt doesn't perform a full reload().

[Hellowlol]

Edit DONT_RELOAD_FOR_KEYS?

[JonnyWong16]

This is intended behaviour as it is designed to reload if an attribute is None. There was some discussion about this in #603.

There is currently no method to identify if an object is partially reloaded. The object is either isPartialObject() or isFullObject(). There is no "PartialFullObject" or "PartialReloadedObject". As long as the object is in the isPartialObject() state then any attribute that is None will cause a full reload since we do not know if the attribute is None because it doesn't exist in the XML (i.e. lastViewedAt), or None because it is a partial object.

I don't think manually keeping a list of attributes that might be missing from the XML is a good idea (e.g. lastViewedAt, originalTitle, etc.). There will just be too many attributes to keep track of and they may be changed by Plex at any time. It would be a huge nightmare to maintain.

A possible solution would be to add a way to check if the object has been partially reloaded (_initpath starts with key but does not contains all the include parameters? or some _partialReload flag?) and don't auto-reload. However, I think automatic reloading should still be the default behaviour. For most users, when they want an attribute, it is more intuitive to have PlexAPI do the reloading in the background and return the actual value to them rather than them asking why an attribute is missing. Maybe we could add some environment variable/config setting to disable auto-reloading after a partial reload (assuming we have a way to flag the partial reload) or just simply disabling auto-reload completely? That way the default behaviour is maintained and anyone that wants to get more advanced with partial reloads can configure it themselves.

[Hellowlol]

I think the current solution is good enough. Advanced users can just add what they want to dont_reload_for_key. We should add a dont_reload_for_value too so advanced user can do what they want without having to subclass.

[Montellese]

Thanks for the helpful explanation and your feedback.

I totally agree that full reloading should be the default but it would be great if there would be a way to disable it manually. Unfortunatelly __getattribute__() is too generic to customize it per call. So either we add a new constant which can be overwritten with a list of attributes to not reload or a way to disable automatic reloading. Or another suitable solution :-)

[Hellowlol]

@Montellese Whats wrong with the constant we already have?

[Montellese]

It didn't sound to me like something that should be changed from the outside. But if that's the intention I'll give it a try.

EDIT: And it contains predefined values so the user has to remember to extend it instead of overwriting it.

[Hellowlol]

@Montellese Did my suggestion work for you?