request: Add title to cleanup routine.

[un1versal]

I thought I could nail this but I cant.

$ python ./texturecache.py vclean
Successfully updated from v2.1.7 to v2.1.8
Cleaning library...
Cleaning library: episodeid  7143 [None]

getting none or just episode id is not really human readable friendly stuff, I tried adding the title part and failed, clearly its above my paygrade.

Can this be added?

[MilhouseVH]

Not without changes in Kodi.

You see, the OnRemove notification is sent by Kodi and this notification only contains the item id of the item being removed, and not the title. So on receipt of an OnRemove notification the script performs a separate JSON query to try and retrieve the title of the item being removed by using the id and this is sometimes successful, but often not - it all depends if the item has been physically deleted from the database in the period between the OnRemove notification being sent and the title query being processed.

If the title is successfully retrieved from Kodi then it will be shown to the user in square brackets after the id. If not then only the id will be shown as that's all we have. I'm not entirely sure why you have [None] being output, as this shouldn't be possible given the current code - do you have a logfile for the run, or can you confirm if your code is modified?

The only reliable solution for this problem would for title to be added to the OnRemove notifications, which would be nice.

[un1versal]

well I changed the code in 2.1.7 to test if my changes did something but no, those chnages would be overridden by 2.1.7 -> 2.1.8 update anyway or no?

Is this something you can request from team?

IF not doesn't matter, its just useless output for humans to look at and I suppose if addons and web interfaces also do that then they get no better human readable output.

[MilhouseVH]

well I changed the code in 2.1.7 to test if my changes did something, that would be overriden by 2.1.8 update anyway or no?

It should have been overwritten, yes... I've looked at the 2.1.8 code on master and don't see how [None] could be output. If you can reproduce and have a log that would be useful.

Is this something you can request from team?

Ping @anaconda / @Montellese

IF not doesn't matter, its just useless output for humans to look at and I suppose if addons and web interfaces also do that then they get no better human readable output.

Hey I like it. :) I understand your point though, but I feel I need to output some sort of progress, even if it's just an id.

[MilhouseVH]

Thinking about this a bit more, and looking at what the current code does, it's actually more than just retrieving title - in the case of an episode the second JSON query will retrieve the tvshow title, the episode title, the season # and episode # and then format a detailed message for the user.

It's probably not practical for all of these fields to be added to the OnRemove notification. Or maybe it is, I dunno.

[un1versal]

see the PR up

This isn't super-easy.
We only fire a OnRemove after the item has been removed from the database (except a couple cases where it's needed for UPnP, but there's a TODO).
The cleaning logic also does only need the item ID and its path, so this would need a separate query (before it's deleted) just for the notification. I don't think this is worth it.
We can move the OnRemove notification before the item is deleted from the database and you may be lucky enough to have the chance to get details in a separate query. This doesn't make sense ("on remove" before it is actually removed?), is an ugly hack and completely unreliable.

[MilhouseVH]

I'm pretty sure there was a time when the OnRemove notification fired before the database delete, leaving a small window of opportunity for the item title to sometimes be grabbed from the database before being physically deleted.

Based on your analysis (many thanks) it would seem a relatively recent refactor has now ensured that the OnRemove always occurs after the database delete - I'm thinking xbmc/xbmc#7306?

I'll probably drop the attempt to obtain the title in a future script update (although leaving it in does no real harm, and would still semi-work on pre-Jarvis builds).

I agree that moving the OnRemove notification before the database delete is not a good solution, although I do wonder how much extra overhead there would be with an extra query? Consider how often things are removed from the library, it's likely to be a relatively rare occurrence.

Although the problem is that to do it properly, when an episode is deleted you'd really need to include the title of the tvshow, plus title of the episode and ideally season # and episode # as fields in the OnRemove notification. For everything else (movie, set, tvshow) just the title of the item being removed would be sufficient.

I can't read.
You're right, it's not "a couple cases where UPnP needs it": in every case (movie, episode, tvshow, musicvideo) OnRemove is fired before the delete:
xbmc/xbmc@5fd21b3 (still there).
Doesn't make any sense for an "on remove" notification, but is interesting and easier.
My idea was to add an item (library item with info tags) parameter to OnRemove so that you get all details.
I'll need to investigate how you get a CFileItem out of a library ID though...

[Montellese]

The idea of notifications is that they are as small as possible so that it doesn't put any unnecessary limitations on the receivers. So expanding the OnRemove notification to contain the whole item with all details is a no-go. Adding only the title could be considered.