Mobileclient remove_entries_from_playlist() always returns the input parameters

[spidru]

I've been using this function to remove playlist entries successfully, but I notice that according to the documentation, the function remove_entries_from_playlist():

Returns a list of entry ids that were removed.

However, it seems that the function will basically return whatever is specified as the input parameter. As a simple example:

api.remove_entries_from_playlist(u'hello')

Will return [u'hello]', where I would expect it to return None since no playlist entry ID exists with this value. Is my understanding correct here?

I'm using version 12.0.0, but from the changelog I believe this also applies for the latest version.

Cheers!

[simon-weber]

I haven't actually run it to see, but at least according to this test it appears to work as expected. Maybe the output is undefined if the entries don't exist? I don't see any code checking that case; it just returns whatever Google does.

[spidru]

From the assert you linked it seems that it is checking that the return value is indeed equal to the input argument specified, which seems to be always true. Perhaps it would be a good idea to also check the inverse (i.e. passing non-existing track IDs causes the return value to be not equal to the input argument)?

[simon-weber]

Right, I'm not saying you're wrong, I just mean the test shows that input == output == removed so long as the input is valid (because the code below the linked spot checks the resulting contents of the playlist). It's not like the function returns the input unmodified -- the actual response from Google just always seems to include it. I have a feeling this is because most of the backend is async, so Google is probably just acknowledging the input without knowing whether it will succeed when applied.

I'd be fine with updating the docs to make it clearer. I'm not sure we'll be able to improve the output itself, though, unless there's more information in the response that we're not currently making use of.

[spidru]

Thanks for the explanation. I agree, it seems the response given by Google is just an acknowledgement of the request. Indeed, I think we could make this clear in the docs.

I think the only way to actually confirm that the playlist entries were removed is by:

1. Checking whether these entries exist in the playlist (perhaps ignoring the ones that don't)
2. Call the Google API to remove the entries from the playlist
3. Check whether these entries still exist in the playlist

However this pre- and post-checking adds some overhead to the function. We could enable this additional check by introducing a new parameter to remove_entries_from_playlist(). I would be happy to do something if you think this is a good idea.

[simon-weber]

I think I'd rather avoid adding some kind of verification, partly because I'm lazy and partly because it would stick out if it wasn't available for other methods as well. Improving the docs would be good, though.