-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
Fix duplicate entries Missing and not Missing #106
Conversation
6762746
to
05a69da
Compare
@BobSilent we just merged in tvdb v4 changes, if this PR is still needed please update |
@crobibero I need to test and check, do I have some time (I think latest until the weekend). A first quick code check shows me that the PR is still required, I will rebase. |
05a69da
to
148e901
Compare
@crobibero I am currently facing some issues with #93, I will prepare a separate PR for these bug fixes! |
148e901
to
feb22ef
Compare
@crobibero a little bit off topic for this PR, but I have prepared another cleanup and I saw this comment here jellyfin-plugin-tvdb/Jellyfin.Plugin.Tvdb/TvdbClientManager.cs Lines 395 to 401 in 0fae371
Are the mentioned "issues upstream" the fact that "suddenly" logging is no longer working as configured? But Serilog is added quite late. if this is changed like that: Shall I create a PR for this in |
@crobibero during testing I saw some issues with the
Post it as separate issue here: crobibero/tvdb-sdk-csharp? |
Thank you for investigating the figuring out the logging issue! We aren't planning on making any more 10.8 releases, so a PR targeting master would be greatly appreciated!
That sdk is 99.97% auto-generated, any schema issues should be filed in the upstream repository (https://github.com/thetvdb/v4-api). The difficulty there lies with trying to figure out which model the issue is actually in I think this PR can be merged without fixing the deserialization issues |
Yes, this is more like a fyi...
Ok, first need to setup a system here on my side, as I saw there are also other (bigger) changes in, like jellyfin/jellyfin#9078 |
Shall I rebase to latest master? |
I'm not sure what you're asking but I'm fairly certain that the issue still persists in latest master of jellyfin/jellyfin |
sorry maybe confusing due too many topics here ;) I just wanted to know if i shall rebase this PR to latest? |
Don't worry about rebasing this PR, I'm going to start reviewing it shortly |
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.
still have some templated logging fixes to be made
This serialization problem is caused by the MovieBaseRecord and possibly MovieExtendedRecord schemas. Should have little effect on the plugin since its scope is limited to Series only. Also, if you can provide the remoteID that causes the problem that will be great. Can't seem to replicate it on my end. |
@scampower3 seriesClient.GetSeriesExtendedAsync(id: 253323, meta: null, @short: true); |
Going to merge this and create an issue to track the deserialization issue |
Just for completeness I tested this on 10.9, here with the changes from jellyfin/jellyfin#9078 this is fixed. |
Thanks for closing the loop! I also tested 10.9 and thought I was going crazy haha Feel free to open a PR here, I can just mark it as blocked until 10.9 is released |
I have the issue as well as described here:
fix: #87
fix: [Issue]: Missing Episode Fetcher Seems to Create Double Season Folders #9999
or here https://forum.jellyfin.org/t-jellyfin-displaying-duplicate-missing-files-for-episodes
https://www.reddit.com/r/jellyfin/comments/t4z9jf/show_missing_episode_showing_episodes_i_already/
improves: Unhelpful error messages #71
mainly if the items were added as missing first, they never get deleted after adding media files.