Skip to content

add missing movie_metainfo, series_metainfo property's#1610

Merged
liiight merged 1 commit intoFlexget:developfrom
Andy2244:series_metainfo
Jan 5, 2017
Merged

add missing movie_metainfo, series_metainfo property's#1610
liiight merged 1 commit intoFlexget:developfrom
Andy2244:series_metainfo

Conversation

@Andy2244
Copy link
Copy Markdown
Contributor

@Andy2244 Andy2244 commented Jan 5, 2017

Motivation for changes:

Some plugins did not support the movie_metainfo, series_metainfo group tag/property.

Detailed changes:

added movie_metainfo to rottentomatoes_lookup
added series_metainfo to thetvdb_lookup, tvmaze_lookup

@Andy2244
Copy link
Copy Markdown
Contributor Author

Andy2244 commented Jan 5, 2017

Regarding this logic, i noticed a small problem. If a metainfo plugin supports multiple id's and no actual plugin for those id's exist than the field name is not retrieved.

Example:
series_identifier_fields = [p.instance.series_identifier for p in plugin.get_plugins(group='series_metainfo')]
This will never get the tvrage_id, so this field still needs to-be hard-coded.

So maybe there should be a extra property or additional list, tuple return of all possible supported id fields. So that we can always get a complete list of all id fields for each category per plugin.

Maybe like this?

    @property
    def series_identifier(self):
        """Returns the plugin main identifier type"""
        return ('tvmaze_id', 'tvrage_id', 'tvdb_id')
or
        return ('tvmaze_id', ['tvrage_id', 'tvdb_id'])
or
        return ['tvmaze_id', 'tvrage_id', 'tvdb_id'])  # index 0 is main id?

or

    @property
    def series_identifier_supported(self):
        """Returns all supported series identifier fields"""
        return ['tvmaze_id', 'tvrage_id', 'tvdb_id']

@liiight
Copy link
Copy Markdown
Member

liiight commented Jan 5, 2017

What metainfo plugin has multiple ids? And tvrage isn't returned because we don't have a tvrage plugin anymore

@Andy2244
Copy link
Copy Markdown
Contributor Author

Andy2244 commented Jan 5, 2017

tvmaze has support for: tvmaze_id, tvdb_id, tvrage_id
tvdb has tvdb_id and fills (imdb_id, zap2it_id) (api supports also more)
tmdb has tmdb_id, imdb (the actual api also supports tvdb, tvrage for series)
trakt has trakt_id, tmdb_id, imdb_id, tvdb_id (api supports also tvrage_id)

So in most cases the remote api's actually support multiple id lookups and also return multiple id's, even if not currently implemented in the plugins.

In my case i wanted to get all possible series id fields that can be used to identify a series and as noted i still have to hardcode tvrage_id.

@liiight
Copy link
Copy Markdown
Member

liiight commented Jan 5, 2017

The idea of this identifier is to have one main identifier per metainfo plugin type which can then be used in various places (like validate allowed values for examples). While obviously the vast majority of plugins return multiple identifiers per successful lookup, it's not relevant for this usage.
This is also why tvrage id is not there and should definitely not be hard-coded in.

@Andy2244
Copy link
Copy Markdown
Contributor Author

Andy2244 commented Jan 5, 2017

I understand this and by hardcode i mean you need to-do something like this atm.

_series_id_fields = [p.instance.series_identifier for p in plugin.get_plugins(group='series_metainfo')] + ['tvrage_id']

I simply thought that adding the ability to get a list of all supported id fields per plugin might be a good enhancement. So you could search for a meta plugin based on a given id field. Lets say a entry has a tvrage_id and now you want to know what meta lookup plugins could be used for a lookup.

@liiight
Copy link
Copy Markdown
Member

liiight commented Jan 5, 2017

The whole point of this feature was to dynamically add support for identifiers via our existing lookup plugins. Anything else is outside the scope of this

@liiight liiight merged commit 5555f26 into Flexget:develop Jan 5, 2017
@Andy2244 Andy2244 deleted the series_metainfo branch January 6, 2017 12:59
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.

2 participants