Opened 8 years ago
Closed 8 years ago
Last modified 8 years ago
#12538 closed Patch - Bug Fix (fixed)
Recent change in tmdb3.py making it often fail on country specific requests
Reported by: | Owned by: | Karl Egly | |
---|---|---|---|
Priority: | minor | Milestone: | 0.27.6 |
Component: | MythTV - Mythmetadatalookup | Version: | 0.27.5 |
Severity: | medium | Keywords: | tmdb3.py |
Cc: | Ticket locked: | no |
Description
While the old code handled a missing country specific release date, the addition of an extra error handler makes it fail. Line 66 to 75:
if opts.country: - try: - # resort releases with selected country at top to ensure it - # is selected by the metadata libraries - index = zip(*releases)[0].index(opts.country) - releases.insert(0, releases.pop(index)) - except IndexError, ValueError: - pass - else: + # resort releases with selected country at top to ensure it + # is selected by the metadata libraries + r = zip(*releases) + if opts.country in r[0]: + index = r[0].index(opts.country) + releases.insert(0, releases.pop(index)) m.releasedate = releases[0][1].releasedate
either this or removing again the IndexError
or reverting the order of the two error handlers. But I think this nicer!
Hika
Change History (12)
comment:1 Changed 8 years ago by
Milestone: | 0.27.6 |
---|---|
Owner: | changed from JYA to Karl Egly |
Status: | new → accepted |
comment:2 Changed 8 years ago by
Status: | accepted → infoneeded |
---|---|
Type: | Bug Report - Crash → Patch - Bug Fix |
Hika, thanks for the patch. I have tested it and while it does fix something it also breaks #12263, again. :(
I have tested with the following movies
- 353127 - has no release date at all - is now broken again
- 153120 - has a german release date - is now working again when you ask for a different countries release date, was broken by the fix for #12263
- 206647 - has lots of release dates - works, too. returns the correct german release date (its not the first/last in the raw data)
I'm not very good in Python (Skill Level: Copy'n'Paste Monkey), so I would appreciate a patch that fixes both use cases :-)
comment:3 Changed 8 years ago by
I'll have a look at it. I only learned Python a year ago, but have had a lot of practice since. I however have to look deeper into the workings of this script first.
comment:4 Changed 8 years ago by
Try changing
except IndexError, ValueError:
to
except (IndexError, ValueError):
comment:5 Changed 8 years ago by
That might work, but I personally find it neater to not use a try loop to catch predictable errors.
OK, the index error comes from an empty result on releases, so change line 68 to:
- if opts.country: + if opts.country and len(releases) > 0:
comment:7 Changed 8 years ago by
I also now see a fundamental flaw as the changed (reordered) release date is later ignored. Probably changing line 78-79 to:
- if movie.releasedate: - m.year = movie.releasedate.year + if m.releasedate: + m.year = m.releasedate.year
will correct it. Or better said, it does, but if opt.country is not set you won't get any releaseyear.
comment:8 Changed 8 years ago by
So sumarizing starting at line 66:
- if opts.country: - try: - # resort releases with selected country at top to ensure it - # is selected by the metadata libraries - index = zip(*releases)[0].index(opts.country) - releases.insert(0, releases.pop(index)) - except IndexError, ValueError: - pass - else: - m.releasedate = releases[0][1].releasedate + if len(releases) > 0: + if opts.country: + # resort releases with selected country at top to ensure it + # is selected by the metadata libraries + r = zip(*releases) + if opts.country in r[0]: + index = r[0].index(opts.country) + releases.insert(0, releases.pop(index)) + + m.releasedate = releases[0][1].releasedate m.inetref = str(movie.id) if movie.collection: m.collectionref = str(movie.collection.id) - if movie.releasedate: - m.year = movie.releasedate.year + if m.releasedate: + m.year = m.releasedate.year
It might be I'm missing some things? Should year be the localized release year or the first release year. It's not all clear to me.
comment:9 Changed 8 years ago by
Oh, and I see a warning from the api, that a date in the database is coming as just a year. ("1969" is not a supported date format. ...) It's not fatal, but it could be caught in the api, converting the year to a date 01-01-<year>. It only needs a test on whether the invalid data consists of four numbers! Just a suggestion. ;-)
comment:10 Changed 8 years ago by
Resolution: | → fixed |
---|---|
Status: | infoneeded → closed |
comment:12 Changed 8 years ago by
Milestone: | → 0.27.6 |
---|
I broke it, I get to keep the pieces. #12263 (edit: wrong reference)