Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use trakt.show_id for episodes #1120

Merged
merged 2 commits into from
Oct 26, 2022
Merged

Use trakt.show_id for episodes #1120

merged 2 commits into from
Oct 26, 2022

Conversation

simonc56
Copy link
Collaborator

@simonc56 simonc56 commented Oct 22, 2022 •
edited
Loading

requires :

Uses the newly added show_id attribute of trakt TVEpisode instead of TVShow id found from Plex show guid.
Usefull when a Plex show is split in many Trakt shows.

closes #1111

@simonc56 simonc56 self-assigned this Oct 22, 2022
@simonc56 simonc56 force-pushed the use-trakt.show_id-for-episodes branch from 9f62806 to 0a00424 Compare October 22, 2022 10:43
@simonc56 simonc56 mentioned this pull request Oct 22, 2022
Closed
2 tasks
@simonc56 simonc56 force-pushed the use-trakt.show_id-for-episodes branch from 0a00424 to 05bcff4 Compare October 23, 2022 16:45
@glensc
Copy link
Collaborator

glensc commented Oct 23, 2022

There are more places where .show is assigned:

$ git grep -F '.show ='
plextraktsync/commands/watch.py:            m.show = ms
plextraktsync/pytrakt_extensions.py:        self.show = show

@glensc glensc force-pushed the use-trakt.show_id-for-episodes branch from 05bcff4 to 6efcba2 Compare October 23, 2022 20:05
@glensc
Copy link
Collaborator

glensc commented Oct 23, 2022

dependency of moogar0880/PyTrakt#208 has been solved in 68ced57

@glensc glensc force-pushed the use-trakt.show_id-for-episodes branch 2 times, most recently from 5cf224a to 655b5d4 Compare October 23, 2022 20:37
@glensc
Copy link
Collaborator

glensc commented Oct 23, 2022 •
edited
Loading

And the show getter/setter added to Media class via #1119 is then also not needed. and once that is removed the media.mf becomes also unused

@glensc
Copy link
Collaborator

glensc commented Oct 23, 2022

Since lazyepisodes is gone, you probably need to add show_id to the block added in:

@glensc
Copy link
Collaborator

glensc commented Oct 23, 2022 •
edited
Loading

perhaps something like

diff --git a/trakt/tv.py b/trakt/tv.py
index e41bf4f..90c7c60 100644
--- a/trakt/tv.py
+++ b/trakt/tv.py
@@ -439,7 +439,7 @@ class TVShow(object):
                 # Prepare episodes
                 episodes = []
                 for ep in season.pop('episodes', []):
-                    episode = TVEpisode(show=self.title, **ep)
+                    episode = TVEpisode(show=self.title, show_id=self.trakt, **ep)
                     episodes.append(episode)
                 season['episodes'] = episodes

glensc
glensc reviewed Oct 23, 2022
View reviewed changes
plextraktsync/trakt_api.py Outdated
@@ -389,7 +389,7 @@ def find_episode_guid(self, guid: PlexGuid, lookup: TraktLookup):
return te.instance
else:
# Retry using search for specific Plex Episode
logger.warning(f"Retry using search for specific Plex Episode {guid.guid}")
logger.debug(f"Retry using search for specific Plex Episode {guid.guid}")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this. if you want to mute the warning, use log filtering:

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it warn the user about this retry ? What is he supposed to do with this info ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't recall anymore why it happened, because data in trakt.tv wrong? or wrong agent?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not asking why it happen. I'm asking why should we warn user about it.
It happen when show order is different between trakt and plex. But the script handles it perfectly now, so the warning has become useless IMHO.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but i am asking why it happens. i recall it was just temporary workaround until real problem is solved.

this fallback is slow (when doing sync) and should be dropped eventually.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and i think it's also unsafe, this code path gets hit when no match is found, but if trakt/plex have different ordering and match is found you would be recording "watched" state to wrong episode.

i don't remember all details anymore.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if trakt/plex have different ordering and match is found you would be recording "watched" state to wrong episode

No, as I said, the script handles different ordering perfectly now, it cannot match a wrong episode anymore. So this warning is useless on user side, that's why i moved it to debug level.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fallback is slow (when doing sync) and should be dropped eventually.

It is needed sometimes. Eg. when a show is split in many shows in trakt.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. keep it as debug then

@glensc
Copy link
Collaborator

glensc commented Oct 24, 2022

the .show is usually needed if you pass the episode object around, to resolve some details (like that Lookup class), need a related show object. maybe could refactor this out by adding to PlexLibraryItem the show property. this will likely simplify tons of code.

@glensc glensc mentioned this pull request Oct 24, 2022
Draft
@simonc56
Copy link
Collaborator Author

the .show is usually needed if you pass the episode object around

Can you show me where .show is needed ?

@simonc56 simonc56 force-pushed the use-trakt.show_id-for-episodes branch from 655b5d4 to 32c5077 Compare October 25, 2022 10:30
@glensc
Copy link
Collaborator

glensc commented Oct 25, 2022

Can you show me where .show is needed ?

I'm not going to answer this 3rd time. remove it and see yourself what breaks!

@simonc56 simonc56 force-pushed the use-trakt.show_id-for-episodes branch from ccfe73b to 9f2a4c6 Compare October 25, 2022 15:39
@simonc56 simonc56 marked this pull request as ready for review October 26, 2022 12:22
@simonc56 simonc56 merged commit 3c38ab4 into Taxel:main Oct 26, 2022
@simonc56 simonc56 deleted the use-trakt.show_id-for-episodes branch October 26, 2022 17:21
@glensc
Copy link
Collaborator

glensc commented Oct 27, 2022 •
edited
Loading

Does this also improve performance? memory usage? overall runtime? api calls?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glensc glensc glensc left review comments

Assignees

@simonc56 simonc56

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Different ordering between Plex and Trakt still not working
2 participants
@simonc56 @glensc