Fix KeyError for video_url, and better workflow #1321
Conversation
|
Sorry for a lame question, but where can I put those lines to make it work? |
|
Attribute |
|
|
aandergr
force-pushed
the
fix_video_url
branch
from
db521b5
to
afb4909
Compare
There was a problem hiding this comment.
Hi! Sorry for my late response, I have been quite busy the last few weeks.
The change looks very good, I have only inline-commented a little suggestion.
I have noticed that one of the Actions failed, fixed that, and rebased your branch on the now-current master.
| if self._context.iphone_support and self._context.is_logged_in: | ||
| try: | ||
| version_urls.extend(version['url'] for version in self._iphone_struct['video_versions']) | ||
| except (InstaloaderException, KeyError, IndexError) as err: | ||
| self._context.error(f"Unable to fetch high-quality video version of {self}: {err}") | ||
| return version_urls[0] | ||
| else: | ||
| version_urls = list(dict.fromkeys(version_urls)) |
There was a problem hiding this comment.
This is to remove duplicates, right? It might be a little cleaner to use a set for version_urls rather than a list. It does not store the order of items, so we will loose the enumeration in the warning message "Video URL candidate 1/4 for ..."; but neither does list(dict.fromkeys(...)) ensure that the order is kept. But that's okay anyway, there is probably little value in printing the index of the URL that fails.
There was a problem hiding this comment.
Yeah, this is to remove duplicates. I prefer this way than set exactly because of the order preservation (even though not very important here), and IIRC since Py 3.5 dicts are ordered, so this operation will keep the order of items.
There was a problem hiding this comment.
Ah interesting, did not know that. Then it is fine. Thanks!
This comment has been minimized.
This comment has been minimized.
Fixes #1320.
Also improves the workflow a little bit.