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.

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

Add imdb, tvdb and tvmaze to feed base release #15245

Merged
merged 3 commits into from Apr 15, 2024

Conversation

Celsiusss
Copy link
Contributor

Description

MTV uses the BaseFeedIndexer. So now imdb, tvdb and tvmaze IDs gets added to MTV releases.

Copy link
Contributor

@ilike2burnthing ilike2burnthing left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution.

Anime Tosho (the other indexer which uses BaseNewznabIndexer) returns the attribute imdb, rather than imdbid (although I'm not sure they actually use it).

The Newznab docs specify that it's imdb, rather than imdbid. As an example, Jackett returns both, imdb for the 'short' (numerical) ID, and imdbid for the full ID (not sure if others do the same). We'd need to be able to handle both.

The only other ID attribute the docs specify is for TVRage, so not TVDB or TVmaze. Either we'd need to stick to the specs here, and then add overrides to the indexers, or we'd need to add all ID attributes here. I'd be leaning towards the former.

@garfield69 / @mynameisbogdan thoughts?

@garfield69
Copy link
Contributor

IMO The basenewsnabindexer should stick to the newsnab specs.
if MTV has added other ids to their service then they are now in custom territory, so the MTV indexer should apply overrides to support these non-newsnab features.

@Celsiusss
Copy link
Contributor Author

@ilike2burnthing @garfield69
Thanks for the feedback.
I moved tvdbid and tvmazeid into the MTV sub class, and kept imdb in the base class.

@mynameisbogdan
Copy link
Contributor

@ilike2burnthing
Copy link
Contributor

Should we rename BaseNewznabIndexer to BaseTorznabIndexer?

@mynameisbogdan
Copy link
Contributor

Not really, except for the download protocol they are pretty much the same.

@ilike2burnthing
Copy link
Contributor

Is there a reason Prowlarr only accepts rageid and not also rage?

Should traktid/trakt be included, as it's not part of the spec?

@mynameisbogdan
Copy link
Contributor

Seems like that was a fix for some indexer.

Prowlarr/Prowlarr#1261

@garfield69
Copy link
Contributor

Apologies to Celsiusss from me, It seems I did not read the current specifications document properly. I jumped to the external references section that pointed to an old newsnab spec https://torznab.github.io/spec-1.3-draft/external/newznab/api.html#predefined-attributes

@garfield69
Copy link
Contributor

As for Traktid, doubanid and tmdbid, I added them to our torznab output along with some categories that were not in the 1.3 specs. I figured since it was unlikely anyone was ever going to go beyond the 1.3 draft specs I might as well add some new ids and cats that were becoming prevalent at some sites, and as long as they did not clash with existing specs they should be be ok.

@ilike2burnthing
Copy link
Contributor

So going forward, do we build on the specs and include Trakt, TMDB, and Douban in BaseNewznabIndexer (and NewznabRssParser), as well as accommodating non-standard attribute names such as tvdb and rage?

@garfield69
Copy link
Contributor

My approach up to now has been to only include a new id only when I discover that there are torznab/newsnab indexers in our stable whose web sites either allow searching for, or present ids in their results that we reprocess into torznab/newsnab output.
What I had not appreciated up to now, was that the torznab 1.3 specs also included an upgrade to the newsnab specs to 1.3 too ;-)

@garfield69
Copy link
Contributor

as for whether to add ids without the id, up to now I had thought that the only variance was imdb, which also had imdbid, one being the number only and the other the ttnumber. All the other ids had the id suffix, and I did not think we should have added the same information under a different name.

@ilike2burnthing
Copy link
Contributor

Prowlarr/Prowlarr#1261 and Prowlarr/Prowlarr#656 were both for newznab indexers, so yea, doesn't really affect us then.

So just rageid, tvdbid, tvmazeid, imdb (short), and imdbid (full) added to BaseNewznabIndexer then?

@garfield69
Copy link
Contributor

yeah

@Celsiusss
Copy link
Contributor Author

Celsiusss commented Apr 15, 2024

Alright great!

@mynameisbogdan
Copy link
Contributor

I did a rebase, so do a git pull.

@garfield69 garfield69 merged commit ddf2a76 into Jackett:master Apr 15, 2024
22 checks passed
@garfield69
Copy link
Contributor

Thank you.

@garfield69
Copy link
Contributor

v0.21.2387

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants