-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
Use history api for importing episodes from trakt #205
Use history api for importing episodes from trakt #205
Conversation
The history api includes more details (i.e. indexer id) which can be used to properly match episodes (i.e. when jellyfin is using other indexer than trakt). Matching by indexer id is a lot more accurate than matching by season and number. This is a rework of jellyfin#191 where we are now using the history by default for episode sync.
If there aren't any specific limits on using the history API I don't see why we should not use it instead of the current implementation. |
There is only 1 call to the history api every time you launch the import from trakt (it was already like that before with my previous PR). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would be the cases in which the fallback would have any results but the history API won't?
I think it's not possible, but not sure. |
If it's not possible, we don't need the fallback and you can remove it alltogether. |
Maybe we can leave it in for now, and check if it's not used by adding some different logging? |
I'd prefer more testing. Just keeping it in because we aren't sure fells kinda dirty to me |
I can test it, but not sure if my library is sufficient as test case... 😄 |
Make distinction between logging for history match and legacy match.
I've changed some logging to see a difference between match by history api or watched api. |
I just did my small test case and it didn't behave quite right. I pull from your PR branch and compiled, installed this on my dev setup.
All my episode have multiple IDs(TVDB,IMDB,etc) as you can see. All watched episodes in question have exactly 1 play recorded on Trakt. Thanks for your work on this. |
To answer the question if it's possible that no provider id exists on an item: yes it is, if the metadata source is a NFO without provider ids. |
This is because of the fallback to the season/number. So it seems it works as expected, the only issue you have is because of the additional fallback. But since it seems that you can have episodes in jellyfin without a provider Id, I don't think we can remove it. Maybe if we do the fallback to season/number matching from history api itself, it would solve your problem. |
Ok, so we need to keep the fallback by season/number. |
@SenorSmartyPants |
It seems that with adding the check for season/number in Below some logs: first 2 don't have id's, other have 1 or more id's
EDIT: there seems to be something wrong with the playcount. |
Ok, I think this should be it. 😉
Play count is now correct. |
This isn't the fix I'm looking for. ;) Same behavior as before. 1x08, 1x14, and 1x28 marked as watched. 1x14 should not be. Just from looking at the log file, it seems like you are iterating over the series in JF and matching them to the history. I think this is why it's giving the false positive match. If you iterate over the history from trakt, and match that to episodes in JF then one history item will only match to one episode in JF. As opposed to now, where one history item could match to mulitple episodes in JF.
BTW, debug lines would be easier for me to read if series name was listed before episode details. |
Please try again 😉 |
Tested before the refactoring. Works right in my test case. One thing that might happen (not sure how often): JF could have ids and Trakt might not. Your test doesn't currently account for that. |
You'd need to explicitly check for the id types. It doesn't help that Trakt only guarantees the tmdb ids (and maybe IMDb) to be correct. Afaik tvdb ids aren't updated anymore. |
I think if TVDB ids are stored on TMDB Trakt gets them. But a possibly bigger problem is that JF doesn't store TMDB ids AFAICT |
I now only match by provider id when both trakt and jellyfin have at least 1 provider id. |
So what you are saying is that we should check if both jellyfin and trakt have the id of the specific type (tvdb, imdb, ...) and only compare when both have it? |
I hope this should be fine now. At least for me it seems to work. |
@Shadowghost Did you already get any change to test this from your side? |
Bump. |
FYI: I've ran the plugin on my whole library and all my missing watched shows are now properly imported.
|
I'm still busy with my thesis, will look at this again once I'm done (~2 weeks). |
Just a regular user here very interested in seeing this released, hoping it fixes the problem I'm seeing as described in #190 In my case, none of the watched movies or shows in my Trakt.tv account are marked as watched in jellyfin on import using the plugin. I'm running Jellyfin.Server 10.8.10.0, with Trakt plug-in 23.0.0.0 in a docker on unraid One thing I don't understand is how to see more information than I do in the below log output, for example any indexer IDs being used and/or what exactly is happening during/after 'ExecuteQueuedTasks' when (presumably) shows are supposed to be getting marked in jellyfin as watched? I also don't understand why I don't seem to see anything in the log about how the plugin settings are enabled such as 'Skip unwatched import from trakt.tv' etc which seems like it would be helpful if not critical to know when trying to debug? In my case I'm using the default out of the box settings. Anyway don't mean to clutter the thread but am appreciative of this work being done to fix the problems and am looking forward to trying it, though as I understand it I wouldn't be able to help test given my docker only environment.
|
@h3llrais3r from the limited testing I did and your discussion in this issue I think this is good to go. |
@Shadowghost I ran it on my complete library and I'm also satisfied with it. It has a better import rate from trakt then before. All the ones that were not imported before are now working. 😉 |
@dabl2 Enable debug logging: https://jellyfin.org/docs/general/administration/troubleshooting/#debug-logging |
Ah thanks. RTFM |
The history api includes more details (i.e. indexer id) which can be used to properly match episodes (i.e. when jellyfin is using other indexer than trakt).
Matching by indexer id is a lot more accurate than matching by season and number.
This is a rework of #191 where we are now using the history by default for episode sync.
@Shadowghost Maybe we can only use the history api for episode import from trakt?
Because I think that the history api will always find a match and there is no need anymore for fallback by season/number lookup.
Please provide your feedback. If you agree, I can cleanup the seaons/number logic for episode import and only import based on history api.
Or we just keep the legacy code as well, in case in future the trakt api for watched shows/episodes would contain the indexer id's.