jellyfin / jellyfin-apiclient-python

Python API Client for Jellyfin
GNU General Public License v3.0
94 stars 32 forks source link

Add get/update item metadata methods #42

Closed Erotemic closed 9 months ago

Erotemic commented 10 months ago

Related to https://github.com/jellyfin/jellyfin-apiclient-python/pull/40 I spent way to much time figuring out how to use the API to update item metadata. Relevant discussion is on #jellyfin-dev:matrix.org on 2024-Jan-02. (If there is a way to link to a matrix conversation, let me know).

There is a jellyfin architecture quirk that makes this implementation less than ideal. It turns out that when you call the UpdateItem endpoint, any data you don't provide is overwritten with null, so we are stuck with one of two bad options:

  1. Force the user to provide all item metadata fields, even if they don't want to update them.
  2. Perform a query before we post the update to construct a well-formed request behind the scenes.

For ease of use, I opted for the second option, but raised another issue: there isn't a an easy API call to get metadata for a single item. There is a get_item, but it doesn't return all of the fields required by the update method. The get_items call does do this, but you have to call it with a list of 1 and then unpack it, which is kinda awkward. I made this problem worse by adding a 3rd method get_item_metadata as a stopgap, but I don't want to merge it and add to API bloat.

Fortunately, this problem can likely be solved by updating the get_item call to pass "Fields": info() in its params dictionary, but I wanted to discuss it first before I did it. Similarly, I'd also like to discuss the name update_item_metadata, which I chose just to let me move forward and modify my library, but it's name doesn't agree with the rest of the API.

My current thoughts are to take the following actions:

Remove get_item_metadata and change get_item to either:

    def get_item(self, item_id):
        return self.users("/Items/%s" % item_id, params={
            'Fields': info()
        })

Or let the user pass in Fields if necessary:

    def get_item(self, item_id, params=None):
        return self.users("/Items/%s" % item_id, params=params)

Then rename update_item_metadata to update_item and keep the same signature.

But there are other directions I could go, so I'd like to discuss before I move further. In the meantime this stop-gap API works well enough for me.

iwalton3 commented 10 months ago

Fortunately, this problem can likely be solved by updating the get_item call to pass "Fields": info() in its params dictionary, but I wanted to discuss it first before I did it.

I think this is reasonable if it doesn't break any existing usages of that function.

Erotemic commented 10 months ago

I made the change. The only way it would break someone is if they wrote code that fails if extra information is returned. The new returned object should always be a superset of what is currently returned on the master branch.