[add] anilist: Include more metadata (al_date_start, al_date_end, mal_id)#3273
[add] anilist: Include more metadata (al_date_start, al_date_end, mal_id)#3273gazpachoking merged 4 commits intoFlexget:developfrom
anilist: Include more metadata (al_date_start, al_date_end, mal_id)#3273Conversation
for more information, see https://pre-commit.ci
flexget/plugins/input/anilist.py
Outdated
| raise plugin.PluginWarning(f'Couldn\'t fetch additional IDs - {e}') | ||
|
|
||
| entry = Entry() | ||
| entry['al_id'] = anime.get('id') or ids.get('anilist') |
There was a problem hiding this comment.
Generally it's better to do
anime.get('id', ids.get('anilist'))
because using or depends how return value of "id" evaluates in boolean context, not if the field is present or not
There was a problem hiding this comment.
Well, I did get scolded once in another project for nesting and chaining get() so I "learned" to favor the or syntax 😅
TBF this fallback isn't even necessary, id is a required field (possibly the only one) as it's the index. This or ends up more like a documentation that the field is available from that second API due to the short-circuit, so I could just comment it instead.
idMAL on the other hand might not be present, but if it is the value is also an integer
There was a problem hiding this comment.
Hahaa, yeah. I am always a bit torn on this subject. But I think the nesting is correct way as using or does not work as expected if value is 0 or "" for example.
There was a problem hiding this comment.
Not completely unexpected, both 0 and "" would be invalid after all xD
|
Looks good to me |
flexget/plugins/input/anilist.py
Outdated
| json={'anilist': anime.get('id')}, | ||
| ).json() | ||
| except RequestException as e: | ||
| ids = {} |
There was a problem hiding this comment.
Why set ids to empty dict here? It's not used ... ?
Also, exception chaining is generally a good idea.
(Besides this PR, holy batman that's a wide exception handing for ValueError)
There was a problem hiding this comment.
If the request isn't successful and ids isn't set then the ids.get() calls below will return exceptions.
There was a problem hiding this comment.
But you're raising PluginWarning immediately after .. which stops the whole plugin execution.
There was a problem hiding this comment.
Didn't realise PluginWarning also stopped execution, thought it just... warned, and PluginError was the one that'd stop execution
|
Feel free to merge this in any time, I think you have the rights :) |
|
I do not. Usually @gazpachoking pushes the button that does the thing xD Side-note: GitHub's diff view doesn't do any favors when lines only have indentation changes 👀 |
Motivation for changes:
Needed the airing dates (start and end) for additional filtering in my config.
Could also use other databases' IDs - AniDB in particular - to optimize Plex matching by automatically generating
anidb2.idfiles for ASS\HAMA upon entry acceptance)Detailed changes:
al_idMalfield for more sensiblemal_idConfig usage if relevant (new plugin or updated schema):